-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[ext] Condense dagster-ext
package to one file
#16321
Conversation
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
dd8276f
to
9549926
Compare
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 9549926. |
Deploy preview for dagster-docs ready! Preview available at https://dagster-docs-4bwno6f5y-elementl.vercel.app Direct link to changed pages: |
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.
not sure about that public API section. Seems like it will just become out of date without any enforcement
# ######################## | ||
# ##### PUBLIC API | ||
# ######################## | ||
|
||
# Defining `__all__` would be redundant for purposes of formalizing the public API, since | ||
# type-checkers will count any non-underscored symbol defined in this module as public. However, it | ||
# is useful to maintain a whitelist of intended-to-be-public symbols for development and | ||
# documentation purposes. | ||
|
||
# ExtContext as ExtContext, | ||
# init_dagster_ext as init_dagster_ext, | ||
# ExtContextLoader as ExtContextLoader, | ||
# ExtBlobStoreMessageWriter as ExtBlobStoreMessageWriter, | ||
# ExtBlobStoreMessageWriterChannel as ExtBlobStoreMessageWriterChannel, | ||
# ExtMessageWriter as ExtMessageWriter, | ||
# ExtMessageWriterChannel as ExtMessageWriterChannel, | ||
# ExtParamLoader as ExtParamLoader, | ||
# ExtDefaultContextLoader as ExtDefaultContextLoader, | ||
# ExtDefaultMessageWriter as ExtDefaultMessageWriter, | ||
# ExtEnvContextLoader as ExtEnvContextLoader, | ||
# ExtS3MessageWriter as ExtS3MessageWriter, | ||
# DAGSTER_EXT_ENV_KEYS as DAGSTER_EXT_ENV_KEYS, | ||
# ExtContextData as ExtContextData, | ||
# ExtDataProvenance as ExtDataProvenance, | ||
# ExtExtras as ExtExtras, | ||
# ExtMessage as ExtMessage, | ||
# ExtParams as ExtParams, | ||
# ExtPartitionKeyRange as ExtPartitionKeyRange, | ||
# ExtTimeWindow as ExtTimeWindow, | ||
# DagsterExtError as DagsterExtError, | ||
# DagsterExtWarning as DagsterExtWarning, | ||
# decode_env_var as decode_env_var, | ||
# encode_env_var as encode_env_var, | ||
# is_dagster_orchestration_active as is_dagster_orchestration_active, |
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.
hmmm very skeptical of the utility of this. almost certainly will fall out of date
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.
fair, it was more useful while I was picking out the methods to underscore than it is now, removed
9549926
to
326d28c
Compare
## Summary & Motivation Condenses `dagster-ext` into a single file `dagster_ext/__init__.py` so it can be vendored easily. I didn't go all the way to just `dagster_ext.py` so that I could leave `dagster_ext/version.py` and `dagster_ext/py.typed`. It should be just as easy to copy-paste the file contents from `__init__.py`. ## How I Tested These Changes Existing test suite.
Summary & Motivation
Condenses
dagster-ext
into a single filedagster_ext/__init__.py
so it can be vendored easily.I didn't go all the way to just
dagster_ext.py
so that I could leavedagster_ext/version.py
anddagster_ext/py.typed
. It should be just as easy to copy-paste the file contents from__init__.py
.How I Tested These Changes
Existing test suite.