Skip to content

Commit

Permalink
Make dagster-images build work when it is called outside of a git repo (
Browse files Browse the repository at this point in the history
#8088)

Summary:
Right now there's a requirement that when you run this command you have the automation library checked out in a github repo. The release pipeline will break that requirement once it is running from a Docker image. So instead figure out where the code is to copy based on the current github repository, rather than assuming a fixed file relative to the python file running the command.

Test Plan:
BK
  • Loading branch information
gibsondan committed May 30, 2022
1 parent 08fad28 commit aa6d4ba
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 13 deletions.
44 changes: 38 additions & 6 deletions python_modules/automation/automation/docker/dagster_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import dagster._check as check

from ..git import git_repo_root
from .ecr import ecr_image, get_aws_account_id, get_aws_region
from .utils import (
execute_docker_build,
Expand All @@ -26,28 +27,57 @@ def do_nothing(_cwd: str) -> Iterator[None]:
yield


def default_images_path():
return os.path.join(
git_repo_root(),
"python_modules",
"automation",
"automation",
"docker",
"images",
)


class DagsterDockerImage(
NamedTuple("_DagsterDockerImage", [("image", str), ("build_cm", Callable), ("path", str)])
NamedTuple(
"_DagsterDockerImage",
[
("image", str),
("images_path", str),
("build_cm", Callable),
],
)
):
"""Represents a Dagster image.
Properties:
image (str): Name of the image
images_path (Optional(str)): The base folder for the images.
build_cm (function): function that is a context manager for build (e.g. for populating a
build cache)
path (Optional(str)): The path to the image's path. Defaults to docker/images/<IMAGE NAME>
"""

def __new__(cls, image: str, build_cm: Callable = do_nothing, path: Optional[str] = None):
def __new__(
cls,
image: str,
images_path: Optional[str] = None,
build_cm: Callable = do_nothing,
):
return super(DagsterDockerImage, cls).__new__(
cls,
check.str_param(image, "image"),
check.callable_param(build_cm, "build_cm"),
check.opt_str_param(
path, "path", default=os.path.join(os.path.dirname(__file__), "images", image)
images_path,
"images_path",
default_images_path(),
),
check.callable_param(build_cm, "build_cm"),
)

@property
def path(self) -> str:
return os.path.join(self.images_path, self.image)

@property
def python_versions(self) -> List[str]:
"""List of Python versions supported for this image."""
Expand Down Expand Up @@ -130,7 +160,9 @@ def _get_docker_args(self, dagster_version: str, python_version: str) -> Dict[st
"BASE_IMAGE" not in docker_args, "Cannot override an existing BASE_IMAGE"
)

base_image = DagsterDockerImage(image_info["base_image"]["name"])
base_image = DagsterDockerImage(
image_info["base_image"]["name"], images_path=self.images_path
)
source = image_info["base_image"]["source"]

if source == "aws":
Expand Down
6 changes: 3 additions & 3 deletions python_modules/automation/automation/docker/image_defs.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import dagster._check as check

from .dagster_docker import DagsterDockerImage
from .dagster_docker import DagsterDockerImage, default_images_path


def get_dagster_repo() -> str:
Expand Down Expand Up @@ -207,12 +207,12 @@ def list_images(images_path: Optional[str] = None) -> List[DagsterDockerImage]:
List[DagsterDockerImage]: A list of all images managed by this tool.
"""

images_path = images_path or os.path.join(os.path.dirname(__file__), "images")
images_path = images_path or default_images_path()
image_folders = [f.name for f in os.scandir(images_path) if f.is_dir()]

images = []
for image in image_folders:
img = DagsterDockerImage(image, path=os.path.join(images_path, image))
img = DagsterDockerImage(image, images_path=images_path)
if image in CUSTOM_BUILD_CONTEXTMANAGERS:
img = img._replace(build_cm=CUSTOM_BUILD_CONTEXTMANAGERS[image])
images.append(img)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ def test_image_path():
"docker",
"images",
)
assert DagsterDockerImage("foo").path == os.path.join(default_images_path, "foo")
assert DagsterDockerImage("foo", path="bar").path == "bar"
assert DagsterDockerImage("foo", default_images_path).path == os.path.join(
default_images_path, "foo"
)
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,13 @@ def test_get_image(tmpdir):
assert get_image("k8s-example")

with pytest.raises(Exception) as e:
get_image("hello-world", images_path=tmpdir)
get_image("hello-world", images_path=str(tmpdir))
assert "could not find image hello-world" in str(e.value)

hello_world = tmpdir / "hello-world"
hello_world.mkdir()
(hello_world / "Dockerfile").write("FROM hello-world")

image = get_image("hello-world", images_path=tmpdir)
image = get_image("hello-world", images_path=str(tmpdir))
assert image.image == "hello-world"
assert image.path == hello_world

0 comments on commit aa6d4ba

Please sign in to comment.