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

[buildkite] stop using str Enum #25874

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

alangenfeld
Copy link
Member

@alangenfeld alangenfeld commented Nov 12, 2024

to enable building the pipeline in newer python versions as well as the current version

python/cpython#100458

How I Tested These Changes

bk

Copy link
Member Author

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

Join @alangenfeld and the rest of your teammates on Graphite Graphite

@alangenfeld alangenfeld force-pushed the al/11-12-_buildkite_stop_using_str_enum branch 4 times, most recently from 085780d to ef58734 Compare November 12, 2024 16:39
@alangenfeld alangenfeld force-pushed the al/11-12-_buildkite_stop_using_str_enum branch from ef58734 to f7cb0a6 Compare November 12, 2024 17:03
Copy link
Member

@gibsondan gibsondan left a comment

Choose a reason for hiding this comment

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

see inline

Comment on lines +67 to 73
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
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Member Author

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

Comment on lines +67 to 73
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
Copy link
Member Author

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,
Copy link
Member Author

Choose a reason for hiding this comment

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

[1]

@alangenfeld alangenfeld merged commit 2941dfc into master Nov 12, 2024
1 check passed
@alangenfeld alangenfeld deleted the al/11-12-_buildkite_stop_using_str_enum branch November 12, 2024 17:21
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.

2 participants