Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(bpdm-gate): Extend error states #985

Conversation

kunyao-cofinity-x
Copy link

@kunyao-cofinity-x kunyao-cofinity-x commented Jul 3, 2024

Description

Enhance the error handling mechanism for the orchestrator and gate components by extending the list of available error codes. This will improve the visibility and detail of error information, particularly beneficial for the customer dashboard to understand why a data set could not be processed.

Currently, the orchestrator and gate components have a limited set of error codes. Extending these error codes will provide more granular information on processing failures, aiding in quicker diagnosis and resolution. This feature will update both the orchestrator and gate components to support a more comprehensive list of error codes.

The new enums add to the BusinessPartnerSharingError:

  • NaturalPersonError - The provided record contains natural person information.
  • BpnErrorNotFound - The provided record can not be matched to a legal entity or an address.
  • BpnErrorTooManyOptions - The provided record can not link to a clear legal entity.
  • MandatoryFieldValidationFailed - The provided record does not fulfill mandatory validation rules.
  • BlacklistCountryPresent - The provided record is part of a country that is not allowed to be processed by the GR process (example: Brazil).
  • UnknownSpecialCharacters - The provided record contains unalloyed special characters.

eclipse-tractusx/sig-release#727

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

@kunyao-cofinity-x kunyao-cofinity-x self-assigned this Jul 3, 2024
@kunyao-cofinity-x kunyao-cofinity-x changed the title Feat/extend error states feat(bpdm-gate): extend error states Jul 3, 2024
@kunyao-cofinity-x kunyao-cofinity-x changed the title feat(bpdm-gate): extend error states feat(bpdm-gate): Extend error states Jul 3, 2024
- now if alternative address is null, the alternative address is also passed as null to the Orchestrator
…e-task-alt-add

Cherry-Pick(release/6.0.x): Fix Gate Sending Alternative Address To Orchestrator
… even when they have no changes

- now business partner service only sets the sharing state to ready or initial when the data really has been changed
…e/input-ready

fix(gate): Starting Golden Record Process Without Changes
… orchestrator

- now made sure sharing states are associated with the tasks in the order of the task create requests
Copy link
Contributor

@nicoprow nicoprow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some doubts in the comments below.

I also expressed these doubts in the references sig-release issue.

Depending on the final implementation maybe some tests are necessary.

BlacklistCountryPresent,
InvalidSpecialCharacters,
MandatoryFieldMissing,
BpnlChanged,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an error code but a comment or success state which we currently do not support in the orchestrator

BpnlChanged,
UnclearEntity,
UnknownSpecialCharacters,
MatchBasedOnProvidedNameOrIdentifier
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also success state here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-added the error codes based on the discussion

ResultState.MatchBasedOnProvidedNameOrIdentifier -> RequestCreationResult.error(
sharingState,
BusinessPartnerSharingError.MatchBasedOnProvidedNameOrIdentifier,
if (task.processingState.errors.isNotEmpty()) task.processingState.errors.joinToString(" // ") { it.description }.take(255) else null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks unnecessarily verbose. Currently, the Gate's sharing state only has up to one error it can display. I would propose to use the generic SharingProcessError type if there are several errors, just like it is currently done. And if there is only one error we can save the more specific error here.

For this I would then use a when-clause mapping from Task error type to Gate error and then map to a RequestCreationResult afterwards.

Proposal somethind like this

fun handleError(...) : RequestCreatinResult{
 if(errors.size == 1){
   when(taskErrorType){
       UnkownSpecialCharacters -> GateErrorType.UnknownSpecialCharacters
       ...
   }.map{ gatErrorType -> RequestCreationResult.erro(...)   }
 }else{
  RequestCreationResult.error(...) like before
 }
}

BpnlChanged,
UnclearEntity,
UnknownSpecialCharacters,
MatchBasedOnProvidedNameOrIdentifier
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Result state indicate a high-level distinction between success, error and in-progress. This is not the place to put specific error types. For this, you can use the TaskErrorType in the same package

nicoprow and others added 11 commits July 8, 2024 14:05
…omcat-core

build(dep): increase tomcat-embed-core dependency version
…e/wrong-task-ids

fix(gate): Assigning Wrong Task-Ids
…and address partners

feat(bpdm-gate): updated upload partner process to only process site and address partners
…artner_upload_endpoint

feat(bpdm-gate): updated upload partner process to only process site and address partners
…-on-release

ci(Linting): Check version increment only on Release
…t-release-check-fix

ci(Release Chart): fix check for Chart release version
nicoprow and others added 4 commits July 11, 2024 12:27
…art/uptime-delay

feat(Charts):  Delay Container Startup
- add jpa entity model
- adapted tests to use entity model where appropriate and setup a test database
- added a postgres dependency to the Orchestrator chart
…ch-persistence-jpa

Feat(Orchestrator): Add persistence to Golden Record Tasks
…e records

- create gate record IDs on receiving task create request which do not already specify a record ID
- Gate now stores received record IDs to each sharing state, so that next time the record ID can be given to the orchestrator
- In the Orchestrator records are identified by both a private and a public record ID. The private record ID is for the Gates to use while the public record ID is sent to the cleaning services
Copy link
Contributor

@SujitMBRDI SujitMBRDI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kunyao-cofinity-x, It looks like rebase on your current branch didn't performed correctly.

@kunyao-cofinity-x
Copy link
Author

kunyao-cofinity-x commented Jul 17, 2024

@kunyao-cofinity-x, It looks like rebase on your current branch didn't performed correctly.

Since the error code needs to be added to the orchestrator table schema(Error Type), that's the reason rebase to the main branch, the branch was created before the orchestrator persistent creating. should I recreate a branch based on the updated main?

@nicoprow
Copy link
Contributor

@kunyao-cofinity-x, It looks like rebase on your current branch didn't performed correctly.

Since the error code needs to be added to the orchestrator table schema(Error Type), that's the reason rebase to the main branch, the branch was created before the orchestrator persistent creating. should I recreate a branch based on the updated main?

In order to make sure that the newest state of main is compatible with the changes we typically request the feature branch to be up-to-date with the newest changes on main.

You can see that you get it right, when this pull request has one commit that it wants to apply to the main branch.

In order to achieve this from your state you will need to use rebasing and/or cherry-picking and update your local main to the newest state of this repository's main

# This is the 1st commit message:

feat: Adding extend error states

# This is the commit message eclipse-tractusx#2:

feat: Adding logic to TaskResolutionService handling conditions

# This is the commit message eclipse-tractusx#3:

feat(bpdm-gate): Extend error states

# This is the commit message eclipse-tractusx#4:

feat(bpdm-gate): Extend error code with description

# This is the commit message eclipse-tractusx#5:

feat(bpdm-gate): Extend error code with description. Extend error code to DB schema

# This is the commit message eclipse-tractusx#6:

Revert "feat(bpdm-gate): Extend error code with description. Extend error code to DB schema"

This reverts commit ff2863d.

# This is the commit message eclipse-tractusx#7:

feat(bpdm-gate): Extend error code with description. Extend error code to DB schema

# This is the commit message eclipse-tractusx#8:

feat: Adding extend error states
author kunyao-cofinity-x <[email protected]> 1721308293 +0200
committer kunyao-cofinity-x <[email protected]> 1721308293 +0200

feat(bpdm-gate): resolve changes

feat(bpdm-gate): Extend error code with description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants