update idempotency key for scheduled runs to disambiguate schedules #17123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CodSpeed Performance ReportMerging #17123 will not alter performanceComparing Summary
|
accept defeat
68c7956 to
4acf27b
Compare
desertaxle
approved these changes
Feb 13, 2025
src/prefect/_result_records.py
Outdated
Comment on lines
147
to
148
| def coerce_old_format(cls, value: dict[str, Any]) -> dict[str, Any]: | ||
| if isinstance(value, dict): # type: ignore[reportUnnecessaryIsInstance] |
There was a problem hiding this comment.
I think it's worthwhile to keep this typing since the value can be Any.
| @@ -149,7 +149,7 @@ | |||
|
|
|||
|
|
|||
| @overload | |||
| def get_client( | |||
| def get_client( # type: ignore # TODO | |||
There was a problem hiding this comment.
This should be fixed when we merge the pyproject migration.
src/prefect/_result_records.py
Outdated
Comment on lines
233
to
235
| if not isinstance(other, ResultRecord): | ||
| return False | ||
| return self.metadata == other.metadata and self.result == other.result | ||
| return self.metadata == other.metadata and self.result == other.result # type: ignore[reportUnknownVariableType] |
There was a problem hiding this comment.
Maybe we can flip the conditional to appease pyright?
if isinstance(other, ResultRecord):
return self.metadata == other.metadata and self.result == other.result
else:
return FalseThere was a problem hiding this comment.
hmm that didn't work, but a model_dump(include did!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #17115
using the MRE in the linked issue we see that we were clobbering scheduled runs bc the idempotency key didn't consider uniqueness wrt deployment schedule id
before
after
also defers fixing
get_clientoverloads and type errors in_result_recordsthat have started failing onmain