dagster-io / dagster

An orchestration platform for the development, production, and observation of data assets.
https://dagster.io
Apache License 2.0
11.54k stars 1.45k forks source link

dagster doesn't automatically refresh pipelines/solids when using a docker volume with docker-compose #4387

Open stuartin opened 3 years ago

stuartin commented 3 years ago

Summary

If we update the example docker_example_pipelines service to use a host volume for the repo.py file, dagster fails to get updated whenever we make changes to the repo.py file on our host, even though I can confirm that the file contents gets updated on the docker_example_pipelines container.

Reproduction

  1. Clone my example repo https://github.com/stuartin/dagster_deploy_docker_issue (changes from the example)
  2. Run docker-compose up -d
  3. Access dagit http://localhost:3000, select my_pipeline, confirm the solid name is hello
  4. Update the repo.py file on the docker host to rename all references of the solid to hello_world
  5. Go back to dagit and click the button the reload the repository
  6. [NOK] The my_pipeline solid is still called hello, I would expect it to get updated to hello_world
  7. Running docker exec docker_example_pipelines /bin/sh -c 'cat repo.py' confirms that the file has been updated

Best case, it would be great if dagster automatically refreshed the contents of a repo/workspace, possibly by adding an additional cli param when launching dagster api grpc with a --poll-interval.

Otherwise I would expect when clicking the button to reload the repository in dagit should refresh the repo.

Dagit UI/UX Issue Screenshots

N/A

Additional Info about Your Environment

$ docker version
Client:
 Cloud integration: 1.0.17
 Version:           20.10.7
 API version:       1.41
 Go version:        go1.16.4
 Git commit:        f0df350
 Built:             Wed Jun  2 12:00:56 2021
 OS/Arch:           windows/amd64
 Context:           default
 Experimental:      true

Server: Docker Engine - Community
 Engine:
  Version:          20.10.7
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.13.15
  Git commit:       b0f5bc3
  Built:            Wed Jun  2 11:54:58 2021
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.4.6
  GitCommit:        d71fcd7d8303cbf684402823e425e9dd2e99285d
 runc:
  Version:          1.0.0-rc95
  GitCommit:        b9ee9c6314599f1b4a7f497e1f1f856fe433d3b7
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Message from the maintainers:

Impacted by this bug? Give it a 👍. We factor engagement into prioritization.

johannkm commented 2 years ago

Hi @stuartin, what dagster version are you seeing this with?

stuartin commented 2 years ago

Hi @stuartin, what dagster version are you seeing this with?

I was testing using the example which looks like it just grabs the latest.

I retested and got the same, below are the versions installed:

docker exec docker_example_pipelines /bin/sh -c 'pip list'
Package                Version
---------------------- ---------
alembic                1.6.5
certifi                2021.10.8
charset-normalizer     2.0.12
click                  8.0.4
coloredlogs            14.0
croniter               1.3.4
dagster                0.14.3
dagster-docker         0.14.3
dagster-postgres       0.14.3
docker                 5.0.3
docker-image-py        0.1.12
docstring-parser       0.13
greenlet               1.1.2
grpcio                 1.44.0
grpcio-health-checking 1.43.0
humanfriendly          10.0
idna                   3.3
importlib-metadata     4.11.2
Jinja2                 2.11.3
Mako                   1.1.6
MarkupSafe             2.0.1
packaging              21.3
pendulum               2.1.2
pep562                 1.1
pip                    21.2.4
protobuf               3.19.4
psycopg2-binary        2.9.3
pyparsing              3.0.7
python-dateutil        2.8.2
python-editor          1.0.4
pytz                   2021.3
pytzdata               2020.1
PyYAML                 6.0
regex                  2022.3.2
requests               2.27.1
Rx                     1.6.1
setuptools             57.5.0
six                    1.16.0
SQLAlchemy             1.4.32
tabulate               0.8.9
toposort               1.7
tqdm                   4.63.0
typing-compat          0.1.0
typing_extensions      4.1.1
urllib3                1.26.8
watchdog               2.1.6
websocket-client       1.3.1
wheel                  0.37.1
zipp                   3.7.0
sapcode commented 1 year ago

Any progress on this topic? We have the same issue where we use nfs3 shares to host the workspace and repository files. The nfs3 share is attached as persistent volume claim to the user-code-deployment pod and dagit shows the initial setup of the repository correctly. However when we change the repo.py and reload the workspace in dagit it's not reflecting new names of operators in the dagit ui.

johannkm commented 1 year ago

This is currently a feature gap in the system- the 'reload' button is pretty misleading when using Docker user code deployments because it reaches back out to the standing server, but that server doesn't pick up the changes. To do so we'll either need to add

sapcode commented 1 year ago

Hi johannkm, thank you for the answer, and yes i realized later when we kill the user code POD, the changes are picked up and visible in dagit after the POD restart. We may use this work arround during CI/CD deployment so that new codebase gets active. Do you see any problem to restart the user code POD diring running pipelines? Does dagster copy the code snapshot into the runner/execution POD so that killing the user POD will not break the execution of running pipelines?

johannkm commented 1 year ago

When using run launchers like the DockerRunLauncher or K8sRunLauncher that spin up isolated containers, there's no dependency on the user code deployment once the run is kicked off. So you're free to spin it down. The only time this isn't true is when using the DefaultRunLauncher which executes runs in subprocesses on the user code deployment container- of course spinning the container down will stop the runs.

dondelicaat commented 1 year ago

Same issue here for copying generated dags into a remote repository. We managed to get the gRPC server to reload data by using subclassing the RepositoryData class since this is not cached by the @repository decorator. However, we would also like to have a kind of --poll-interval flag to keep Dagit synced automatically.