Page MenuHomePhabricator
Differential D4752

Add postgres DB container to celery docker test, fix api call escaping
ClosedPublic
Actions

Authored by dgibson on Mon, Oct 12, 7:02 PM.

Details

Reviewers
johann
alangenfeld
Commits
R1:c9aae7bc8c05: Add postgres DB container to celery docker test, fix api call escaping
Test Plan

BK

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dgibson created this revision.Mon, Oct 12, 7:02 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Mon, Oct 12, 7:18 PM
Harbormaster failed remote builds in B19404: Diff 23579!
dgibson updated this revision to Diff 23594.Tue, Oct 13, 1:10 AM

up

dgibson updated this revision to Diff 23595.Tue, Oct 13, 1:12 AM

lint

Harbormaster failed remote builds in B19416: Diff 23594!Tue, Oct 13, 1:29 AM
dgibson updated this revision to Diff 23598.Tue, Oct 13, 1:29 AM

lint

dgibson updated this revision to Diff 23599.Tue, Oct 13, 1:31 AM

up, include johann's original diff too

Miraculously BK seems to be passing??

Harbormaster failed remote builds in B19417: Diff 23595!Tue, Oct 13, 1:32 AM
dgibson updated this revision to Diff 23600.Tue, Oct 13, 1:36 AM

up

Harbormaster completed remote builds in B19420: Diff 23598.Tue, Oct 13, 1:47 AM
Harbormaster completed remote builds in B19421: Diff 23599.
dgibson added reviewers: johann, alangenfeld.Tue, Oct 13, 1:51 AM
Harbormaster completed remote builds in B19422: Diff 23600.Tue, Oct 13, 1:52 AM
dgibson requested review of this revision.Tue, Oct 13, 1:52 AM
alangenfeld added a comment.Tue, Oct 13, 3:22 PM

nice

python_modules/dagster-test/dagster_test/test_project/__init__.py
45

can you delete the other copy now that this one is more central?

python_modules/libraries/dagster-celery-docker/dagster_celery_docker/executor.py
61–62

we can deprecate / remove this

223

*

229

resolve

dgibson added inline comments.Tue, Oct 13, 3:23 PM
python_modules/dagster-test/dagster_test/test_project/__init__.py
45

it's not a copy, that is an ExternalPipeline, this is a ReconstructablePipeline. We could consolidate their locations though?

alangenfeld added inline comments.Tue, Oct 13, 3:38 PM
python_modules/dagster-test/dagster_test/test_project/__init__.py
45

ya lets keep our "yikes" together

dgibson planned changes to this revision.Tue, Oct 13, 6:03 PM
dgibson updated this revision to Diff 23633.Tue, Oct 13, 6:29 PM

feedback

Harbormaster completed remote builds in B19449: Diff 23633.Tue, Oct 13, 6:47 PM
dgibson updated this revision to Diff 23639.Tue, Oct 13, 7:04 PM

up

Harbormaster completed remote builds in B19455: Diff 23639.Tue, Oct 13, 7:24 PM
alangenfeld accepted this revision.Tue, Oct 13, 7:27 PM

maybe add a CHANGES.md entry about the breaking config changes in this diff so it doesn't get missed

This revision is now accepted and ready to land.Tue, Oct 13, 7:27 PM
dgibson mentioned this in D4769: Use execute_step_with_structured_logs in DagsterDockerOperator.Tue, Oct 13, 8:32 PM
dgibson updated this revision to Diff 23743.Thu, Oct 15, 12:48 PM

up

Harbormaster completed remote builds in B19544: Diff 23743.Thu, Oct 15, 1:06 PM
dgibson updated this revision to Diff 23746.Thu, Oct 15, 1:29 PM

raise an error if nothing comes back from the command

Harbormaster completed remote builds in B19546: Diff 23746.Thu, Oct 15, 1:47 PM
Closed by commit R1:c9aae7bc8c05: Add postgres DB container to celery docker test, fix api call escaping (authored by johann, committed by dgibson). · Explain WhyThu, Oct 15, 2:07 PM
This revision was automatically updated to reflect the committed changes.
dgibson added a commit: R1:c9aae7bc8c05: Add postgres DB container to celery docker test, fix api call escaping.
dgibson mentioned this in R1:febffd667ebb: Use execute_step_with_structured_logs in DagsterDockerOperator.Mon, Oct 19, 7:40 PM

Revision Contents
Changeset List

PathSize
integration_tests/
python_modules/
dagster-k8s-test-infra/
dagster_k8s_test_infra/
32 lines
test_suites/
celery-k8s-integration-test-suite/
2 lines
k8s-integration-test-suite/
2 lines
7 lines
python_modules/
dagster-test/
dagster_test/
test_project/
78 lines
libraries/
dagster-celery-docker/
dagster_celery_docker/
113 lines
dagster_celery_docker_tests/
20 lines
59 lines
3 lines

Diff 23748

View Options

integration_tests/python_modules/dagster-k8s-test-infra/dagster_k8s_test_infra/integration_utils.py

Loading...
View Options

integration_tests/test_suites/celery-k8s-integration-test-suite/test_integration.py

Loading...
View Options

integration_tests/test_suites/k8s-integration-test-suite/test_integration.py

Loading...
View Options

integration_tests/test_suites/k8s-integration-test-suite/test_job_spec.py

Loading...
View Options

python_modules/dagster-test/dagster_test/test_project/__init__.py

Loading...
View Options

python_modules/libraries/dagster-celery-docker/dagster_celery_docker/executor.py

Loading...
View Options

python_modules/libraries/dagster-celery-docker/dagster_celery_docker_tests/docker-compose.yml

Loading...
View Options

python_modules/libraries/dagster-celery-docker/dagster_celery_docker_tests/test_execute_docker.py

Loading...
View Options

python_modules/libraries/dagster-celery-docker/tox.ini

Loading...