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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions .buildkite/dagster-buildkite/dagster_buildkite/package_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,13 @@ def build_steps(self) -> List[BuildkiteTopLevelStep]:
if v not in unsupported_python_versions
]

pytest_python_versions = sorted(
list(set(default_python_versions) - set(unsupported_python_versions))
)
pytest_python_versions = [
AvailablePythonVersion(v)
for v in sorted(
set(e.value for e in default_python_versions)
- set(e.value for e in unsupported_python_versions)
)
]
# Use highest supported python version if no defaults_match
if len(pytest_python_versions) == 0:
pytest_python_versions = [supported_python_versions[-1]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from dagster_buildkite.utils import is_release_branch, safe_getenv


class AvailablePythonVersion(str, Enum):
class AvailablePythonVersion(Enum):
# Ordering is important here, because some steps will take the highest/lowest available version.
V3_9 = "3.9"
V3_10 = "3.10"
Expand Down Expand Up @@ -60,6 +60,6 @@ def from_major_minor(cls, major: int, minor: int) -> "AvailablePythonVersion":

@classmethod
def to_tox_factor(cls, version: "AvailablePythonVersion") -> str:
ver_parts = version.split(".")
ver_parts = version.value.split(".")
major, minor = ver_parts[0], ver_parts[1]
return f"py{major}{minor}"
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def on_test_image(
raise Exception(f"Unsupported python version for test image: {ver}.")

return self.on_python_image(
image=f"buildkite-test:py{ver}-{BUILDKITE_TEST_IMAGE_VERSION}",
image=f"buildkite-test:py{ver.value}-{BUILDKITE_TEST_IMAGE_VERSION}",
env=env,
)

Expand Down
24 changes: 14 additions & 10 deletions .buildkite/dagster-buildkite/dagster_buildkite/steps/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
GCP_CREDS_LOCAL_FILE,
LATEST_DAGSTER_RELEASE,
)
from dagster_buildkite.package_spec import PackageSpec, UnsupportedVersionsFunction
from dagster_buildkite.package_spec import (
PackageSpec,
PytestExtraCommandsFunction,
UnsupportedVersionsFunction,
)
from dagster_buildkite.python_version import AvailablePythonVersion
from dagster_buildkite.step_builder import BuildkiteQueue
from dagster_buildkite.steps.test_project import test_project_depends_fn
Expand Down Expand Up @@ -60,12 +64,12 @@ def build_backcompat_suite_steps() -> List[BuildkiteTopLevelStep]:
)


def backcompat_extra_cmds(_, factor: str) -> List[str]:
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
Comment on lines +67 to 73
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

webserver_library_version = _get_library_version(webserver_version)
user_code_version = tox_factor_map[factor]
Expand Down Expand Up @@ -163,7 +167,7 @@ def build_auto_materialize_perf_suite_steps():

def daemon_pytest_extra_cmds(version: AvailablePythonVersion, _):
return [
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version,
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version.value,
'export DAGSTER_DOCKER_REPOSITORY="$${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com"',
"pushd integration_tests/test_suites/daemon-test-suite/monitoring_daemon_tests/",
"docker-compose up -d --remove-orphans",
Expand All @@ -182,7 +186,7 @@ def daemon_pytest_extra_cmds(version: AvailablePythonVersion, _):
# ########################


def build_k8s_suite_steps():
def build_k8s_suite_steps() -> List[BuildkiteTopLevelStep]:
pytest_tox_factors = ["-default", "-subchart"]
directory = os.path.join("integration_tests", "test_suites", "k8s-test-suite")
return build_integration_suite_steps(
Expand All @@ -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]

queue=None,
always_run_if: Optional[Callable[[], bool]] = None,
unsupported_python_versions: Optional[
Expand Down Expand Up @@ -229,19 +233,19 @@ def build_integration_suite_steps(
).build_steps()


def k8s_integration_suite_pytest_extra_cmds(version: str, _) -> List[str]:
def k8s_integration_suite_pytest_extra_cmds(version: AvailablePythonVersion, _) -> List[str]:
return [
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version,
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version.value,
'export DAGSTER_DOCKER_REPOSITORY="$${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com"',
"aws ecr get-login --no-include-email --region us-west-2 | sh",
]


def celery_k8s_integration_suite_pytest_extra_cmds(version: str, _) -> List[str]:
def celery_k8s_integration_suite_pytest_extra_cmds(version: AvailablePythonVersion, _) -> List[str]:
cmds = [
'export AIRFLOW_HOME="/airflow"',
"mkdir -p $${AIRFLOW_HOME}",
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version,
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version.value,
'export DAGSTER_DOCKER_REPOSITORY="$${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com"',
"aws ecr get-login --no-include-email --region us-west-2 | sh",
]
Expand Down
16 changes: 8 additions & 8 deletions .buildkite/dagster-buildkite/dagster_buildkite/steps/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def _get_uncustomized_pkg_roots(root: str, custom_pkg_roots: List[str]) -> List[
# ########################


def airflow_extra_cmds(version: str, _) -> List[str]:
def airflow_extra_cmds(version: AvailablePythonVersion, _) -> List[str]:
return [
'export AIRFLOW_HOME="/airflow"',
"mkdir -p $${AIRFLOW_HOME}",
Expand Down Expand Up @@ -160,9 +160,9 @@ def dagster_graphql_extra_cmds(_, tox_factor: Optional[str]) -> List[str]:
]


def celery_extra_cmds(version: str, _) -> List[str]:
def celery_extra_cmds(version: AvailablePythonVersion, _) -> List[str]:
return [
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version,
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version.value,
'export DAGSTER_DOCKER_REPOSITORY="$${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com"',
"pushd python_modules/libraries/dagster-celery",
# Run the rabbitmq db. We are in docker running docker
Expand All @@ -178,7 +178,7 @@ def celery_extra_cmds(version: str, _) -> List[str]:
]


def celery_docker_extra_cmds(version: str, _) -> List[str]:
def celery_docker_extra_cmds(version: AvailablePythonVersion, _) -> List[str]:
return celery_extra_cmds(version, _) + [
"pushd python_modules/libraries/dagster-celery-docker/dagster_celery_docker_tests/",
"docker-compose up -d --remove-orphans",
Expand All @@ -192,9 +192,9 @@ def celery_docker_extra_cmds(version: str, _) -> List[str]:
]


def docker_extra_cmds(version: str, _) -> List[str]:
def docker_extra_cmds(version: AvailablePythonVersion, _) -> List[str]:
return [
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version,
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version.value,
'export DAGSTER_DOCKER_REPOSITORY="$${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com"',
"pushd python_modules/libraries/dagster-docker/dagster_docker_tests/",
"docker-compose up -d --remove-orphans",
Expand Down Expand Up @@ -230,9 +230,9 @@ def docker_extra_cmds(version: str, _) -> List[str]:
]


def k8s_extra_cmds(version: str, _) -> List[str]:
def k8s_extra_cmds(version: AvailablePythonVersion, _) -> List[str]:
return [
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version,
"export DAGSTER_DOCKER_IMAGE_TAG=$${BUILDKITE_BUILD_ID}-" + version.value,
'export DAGSTER_DOCKER_REPOSITORY="$${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com"',
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,16 @@ def build_test_project_steps() -> List[GroupStep]:
" $${GOOGLE_APPLICATION_CREDENTIALS}",
"export"
" BASE_IMAGE=$${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/test-project-base:py"
+ version
+ version.value
+ "-"
+ TEST_PROJECT_BASE_IMAGE_VERSION,
# build and tag test image
"export"
" TEST_PROJECT_IMAGE=$${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/test-project:$${BUILDKITE_BUILD_ID}-"
+ version,
+ version.value,
"git config --global --add safe.directory /workdir",
"./python_modules/dagster-test/dagster_test/test_project/build.sh "
+ version
+ version.value
+ " $${TEST_PROJECT_IMAGE}",
#
# push the built image
Expand All @@ -67,7 +67,7 @@ def build_test_project_steps() -> List[GroupStep]:
)
.on_python_image(
# py version can be bumped when rebuilt
f"buildkite-build-test-project-image:py{AvailablePythonVersion.V3_11}-{BUILDKITE_BUILD_TEST_PROJECT_IMAGE_IMAGE_VERSION}",
f"buildkite-build-test-project-image:py{AvailablePythonVersion.V3_11.value}-{BUILDKITE_BUILD_TEST_PROJECT_IMAGE_IMAGE_VERSION}",
[
"AIRFLOW_HOME",
"AWS_ACCOUNT_ID",
Expand Down