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

Gracefully handle API call failure in cluster-events #1285

Open
wants to merge 2 commits into
base: 0820_release
Choose a base branch
from

Conversation

souravbaner-da
Copy link
Contributor

Gracefully handle API call failure in cluster-events

Solve #1284

@souravbaner-da souravbaner-da added the bug Something isn't working label Sep 9, 2024
@souravbaner-da souravbaner-da added this to the 0.8.2.0 milestone Sep 9, 2024
@souravbaner-da souravbaner-da self-assigned this Sep 9, 2024
@neilbest-db neilbest-db linked an issue Sep 17, 2024 that may be closed by this pull request
Copy link
Contributor

@neilbest-db neilbest-db left a comment

Choose a reason for hiding this comment

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

Would it be better from a separation-of-concerns perspective to simply allow this module to have the same EMPTY status message as any other module but indicate that there were API errors in the status of the upstream module? if we are already doing that then it should be adequate unless we want to consider preventing downstream modules from proceeding in that scenario . . . What's our established pattern in that respect?

An alternative to consider would be to automatically check for relevant records in ${erroredBronzeEventsTarget.tableFullName} at runtime rather than expecting the user to notice this advice in the pipReport.

@neilbest-db neilbest-db modified the milestones: 0.8.2.0, 0.9.0.0 Sep 17, 2024
Copy link
Contributor

@neilbest-db neilbest-db left a comment

Choose a reason for hiding this comment

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

Did you say that you are still working on more changes? Maybe you have some commits to push? LMK how I can help.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please see this comment on the PR.

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Edge case for 1002 and 1005 failing Api call
3 participants