-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add deprecation warnings to results classes and methods #15323
Conversation
CodSpeed Performance ReportMerging #15323 will not alter performanceComparing Summary
|
@deprecated.deprecated_callable( | ||
start_date="Sep 2024", | ||
end_date="Nov 2024", | ||
help="Use `create_result_record` instead.", | ||
) | ||
@sync_compatible | ||
async def create_result( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecation warning for create_reuslt
@deprecated.deprecated_class( | ||
start_date="Sep 2024", end_date="Nov 2024", help="Use `ResultRecord` instead." | ||
) | ||
class PersistedResult(BaseResult): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecation warning for PersistedResult
@deprecated.deprecated_class( | ||
start_date="Sep 2024", end_date="Nov 2024", help="Use `ResultRecord` instead." | ||
) | ||
@register_base_type | ||
class BaseResult(BaseModel, abc.ABC, Generic[R]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecation warning for BaseResult
def should_reraise_warning(warning): | ||
""" | ||
Determine if a deprecation warning should be reraised based on the date. | ||
|
||
Deprecation warnings that have passed the date threshold should be reraised to | ||
ensure the deprecated code paths are removed. | ||
""" | ||
message = str(warning.message) | ||
try: | ||
# Extract the date from the new message format | ||
date_str = message.split("not be available in new releases after ")[1].strip( | ||
"." | ||
) | ||
# Parse the date | ||
deprecation_date = datetime.strptime(date_str, "%b %Y").date().replace(day=1) | ||
|
||
# Check if the current date is after the start of the month following the deprecation date | ||
current_date = datetime.now().date().replace(day=1) | ||
return current_date > deprecation_date | ||
except Exception: | ||
# Reraise in cases of failure | ||
return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this check to know when the deprecated code is ready to be removed. As a result, code path that raise deprecation warnings in tests will need to use the ignore_prefect_deprecation_warnings
fixture directly.
This PR adds deprecation warning to following because they have been superseded by new results mechanics:
ResultStore.create_result
BaseResult
PersistedResult
This PR also updates our testing logic to fail whenever a
PrefectDeprecationWarning
is raised. A fixtureignore_prefect_deprecation_warnings
can suppress failures until the deprecation period has passed. Once the deprecation period has passed, the warnings will cause test failures again, prompting us to remove the deprecated code.Related to #15208
Checklist
<link to issue>
"mint.json
.