This was a very good point on https://github.com/dagster-io/dagster/pull/4331 - if you specify a secret on the run launcher, secrets on the executor should be additive, not... replace-i-tive.
Details
Details
Integration
Diff Detail
Diff Detail
- Repository
- R1 dagster
- Branch
- addstuff (branched from master)
- Lint
Lint Passed - Unit
No Test Coverage
Event Timeline
Comment Actions
Should dedup the the lists? Or at least test behavior with duplicates (I think that's already happening in the integration test?)
| integration_tests/test_suites/k8s-integration-test-suite/test_executor.py | ||
|---|---|---|
| 136 | What executor config are we leaving out for this test? | |
Comment Actions
@alangenfeld or @rexledesma if you wouldn't mind reviewing this since johann is out that week I would be very grateful
| python_modules/libraries/dagster-k8s/dagster_k8s/executor.py | ||
|---|---|---|
| 86–93 | the or [] is a bit odd - exc_cfg.get should return a list since it's typed by config, right? | |
| python_modules/libraries/dagster-k8s/dagster_k8s/executor.py | ||
|---|---|---|
| 86–93 | env_config_maps is Noneable so I believe it can be None, for better or worse | |
Comment Actions
(was more relevant when it would replace the launcher config when it was set though - since in that case None was distinct from [])