-
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
[buildkite] stop using str Enum #25874
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @alangenfeld and the rest of your teammates on Graphite |
085780d
to
ef58734
Compare
ef58734
to
f7cb0a6
Compare
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.
see inline
def backcompat_extra_cmds(_, factor: Optional[str]) -> List[str]: | ||
tox_factor_map = { | ||
"user-code-latest-release": LATEST_DAGSTER_RELEASE, | ||
"user-code-earliest-release": EARLIEST_TESTED_RELEASE, | ||
} | ||
|
||
assert factor | ||
webserver_version = DAGSTER_CURRENT_BRANCH |
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.
?
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.
aligning the typing [1] makes factor
Optional
but we must always be getting it in this context so i just asserted to make the type checker happy
def backcompat_extra_cmds(_, factor: Optional[str]) -> List[str]: | ||
tox_factor_map = { | ||
"user-code-latest-release": LATEST_DAGSTER_RELEASE, | ||
"user-code-earliest-release": EARLIEST_TESTED_RELEASE, | ||
} | ||
|
||
assert factor | ||
webserver_version = DAGSTER_CURRENT_BRANCH |
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.
aligning the typing [1] makes factor
Optional
but we must always be getting it in this context so i just asserted to make the type checker happy
@@ -201,7 +205,7 @@ def build_k8s_suite_steps(): | |||
def build_integration_suite_steps( | |||
directory: str, | |||
pytest_tox_factors: Optional[List[str]], | |||
pytest_extra_cmds: Optional[Callable] = None, | |||
pytest_extra_cmds: Optional[PytestExtraCommandsFunction] = None, |
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.
[1]
to enable building the pipeline in newer python versions as well as the current version
python/cpython#100458
How I Tested These Changes
bk