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

[utils] add preview warning and decorator #26747

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

maximearmstrong
Copy link
Contributor

@maximearmstrong maximearmstrong commented Dec 27, 2024

Summary & Motivation

Fixes AD-735 in the API Lifecycle project.

Adding new Preview 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.

Copy link
Contributor Author

maximearmstrong commented Dec 27, 2024

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

@maximearmstrong maximearmstrong marked this pull request as ready for review December 27, 2024 21:19
@maximearmstrong maximearmstrong self-assigned this Dec 27, 2024
return

warnings.warn(
f"{subject} is a preview in early testing phase with frequent changes, not recommended for production use."
Copy link
Contributor

Choose a reason for hiding this comment

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

i know this is the original language we had in the doc. but when it's putting in a user-facing warning like this, it feels a little scary. how about tuning it down:

is a preview currently in early testing, with ongoing updates, and not yet optimized for production use.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the "production use" verbiage is okay here, but just swinging back from the upstack review, maybe:

is currently in preview, with ongoing updates. This may break in future versions, even between dot releases.

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.

agreed w/ Yuhan re: the verbiage, otherwise seems good to me

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.

requesting changes for queue management

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants