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.

Sign up for GitHub

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

Jump to bottom

[utils] add beta warning and decorator #26748

Merged
merged 4 commits into from
Jan 3, 2025
Merged

[utils] add beta warning and decorator #26748

merged 4 commits into from
Jan 3, 2025
+141 −0

Conversation

maximearmstrong
Copy link
Contributor

@maximearmstrong maximearmstrong commented Dec 27, 2024 •
edited
Loading

Summary & Motivation

Fixes AD-736 in the API Lifecycle project.

Adding new Beta Python utils. Similar to #25363

This is done in alignment with the new API lifecycle.

The raised warning is a Warning.

Docs to be updated in subsequent PR.

@maximearmstrong Graphite App
Copy link
Contributor Author

maximearmstrong commented Dec 27, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@maximearmstrong maximearmstrong mentioned this pull request Dec 27, 2024
Merged
@maximearmstrong maximearmstrong force-pushed the maxime/ad-736/add-beta-annotation branch from 351fbe0 to 5f95dcf Compare December 27, 2024 21:15
@maximearmstrong maximearmstrong self-assigned this Dec 27, 2024
@maximearmstrong maximearmstrong marked this pull request as ready for review December 27, 2024 21:20
yuhan
yuhan reviewed Dec 27, 2024
View reviewed changes
python_modules/dagster/dagster/_utils/warnings.py Outdated
return

warnings.warn(
f"{subject} is in beta stage - the feature is still being tested and may change."
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems ok - take it or leave it

is in beta stage - the feature is being tested for production readiness and may be updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

is in beta. It may break in a future minor release, or have minor behavior changes between dot releases.

I don't love this "production readiness" framing, as I think the key important detail here is when / how likely this feature is to be broken, and what those breakages might look like

In my mind:

  • preview: might break even between dot releases
  • beta: might break on a dot release, or have behavior changes

Copy link
Contributor Author

@maximearmstrong maximearmstrong Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I agree. Updated in 0f9af0a to

is in beta. It may break in a future minor release, or have minor behavior changes between dot releases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 40b7310 after receiving feedback in this thread

is currently in beta, and may have breaking changes in minor version releases, with behavior changes in patch releases.

OwenKephart
OwenKephart previously requested changes Dec 31, 2024
View reviewed changes
Copy link
Contributor

@OwenKephart OwenKephart left a comment

Choose a reason for hiding this comment

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

comment

@maximearmstrong maximearmstrong force-pushed the maxime/ad-735/add-preview-annotation branch from d472b02 to a2e7976 Compare January 2, 2025 15:26
@maximearmstrong maximearmstrong force-pushed the maxime/ad-736/add-beta-annotation branch from 5f95dcf to 0f9af0a Compare January 2, 2025 15:26
@maximearmstrong maximearmstrong force-pushed the maxime/ad-736/add-beta-annotation branch from 0f9af0a to 2bec9f1 Compare January 2, 2025 15:38
@maximearmstrong maximearmstrong force-pushed the maxime/ad-735/add-preview-annotation branch from 71d62fd to 9734765 Compare January 2, 2025 17:34
@maximearmstrong maximearmstrong force-pushed the maxime/ad-736/add-beta-annotation branch from 2bec9f1 to 40b7310 Compare January 2, 2025 17:35
@maximearmstrong maximearmstrong requested review from PedramNavid and removed request for danielgafni January 2, 2025 17:39
cmpadden
cmpadden approved these changes Jan 3, 2025
View reviewed changes
Copy link
Contributor

@cmpadden cmpadden left a comment

Choose a reason for hiding this comment

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

Messaging looks good given conversation in Slack.

Base automatically changed from maxime/ad-735/add-preview-annotation to master January 3, 2025 15:32
@maximearmstrong maximearmstrong force-pushed the maxime/ad-736/add-beta-annotation branch from 40b7310 to ed94453 Compare January 3, 2025 15:32
@maximearmstrong maximearmstrong merged commit 5a63f7e into master Jan 3, 2025
1 check passed
@maximearmstrong maximearmstrong deleted the maxime/ad-736/add-beta-annotation branch January 3, 2025 18:48
@maximearmstrong maximearmstrong mentioned this pull request Jan 3, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmpadden cmpadden cmpadden approved these changes

@yuhan yuhan Awaiting requested review from yuhan

@OwenKephart OwenKephart Awaiting requested review from OwenKephart

@PedramNavid PedramNavid Awaiting requested review from PedramNavid

Assignees

@maximearmstrong maximearmstrong

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@maximearmstrong @yuhan @cmpadden @OwenKephart