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

fix(results-store): catch and handle any Exception from exists() #13990

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

parkedwards
Copy link
Contributor

@parkedwards parkedwards commented Jun 12, 2024

resolves #13989

@desertaxle and I were discussing ^, and it looks like we might be running into a problem with remote result_store uses if we expect this exists() check to fire on every task run start. If we don't catch every vendor-specific Exception, it seems like we might prevent those tasks from starting - this is something I observed with the GcsBucket() results store

Example

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • This pull request includes a label categorizing the change e.g. maintenance, fix, feature, enhancement, docs.

For documentation changes:

  • This pull request includes redirect settings in mint.json for files that are removed or renamed.

For new functions or classes in the Python SDK:

  • This pull request includes helpful docstrings.
  • If a new Python file was added, this pull request contains a stub page in the Python SDK docs and an entry in docs/mint.json navigation.

@parkedwards parkedwards requested a review from a team as a code owner June 12, 2024 21:02
@parkedwards parkedwards requested review from cicdw and removed request for a team June 12, 2024 21:02
@parkedwards
Copy link
Contributor Author

hey @cicdw - wanted to get your opinion on this. is there a footgun here / am I missing something obvious

Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

Yup, totally agreed - nice catch and thanks for fixing it!!

@parkedwards parkedwards added fix A fix for a bug in an existing feature 3.x labels Jun 12, 2024
@parkedwards parkedwards merged commit c9ecf59 into main Jun 12, 2024
30 checks passed
@parkedwards parkedwards deleted the fix/result-store-exception branch June 12, 2024 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x fix A fix for a bug in an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remote result_storage setting is preventing task from starting from missing (random) key
2 participants