Skip to content

fix: support docker/.env-local for docker-compose#28039

Merged
mistercrunch merged 2 commits into
masterfrom
backtrack-env-local
Apr 15, 2024
Merged

fix: support docker/.env-local for docker-compose#28039
mistercrunch merged 2 commits into
masterfrom
backtrack-env-local

Conversation

@mistercrunch
Copy link
Copy Markdown
Member

@mistercrunch mistercrunch commented Apr 15, 2024

As reported here #28026, in #27953 I added support for a supplemental docker/.env-local override file, not thinking it was going to be required upon starting docker-compose. I thought I had tested that while switching branches but somehow didn't test that docker-compose would simply boot up in the absence of that file.

BUT! luckily there's a new feature as of docker-compose 2.24.0 where there's a way to specify required and not-required files here, so I'm taking advantage of that in this correction PR. Kudos to @artofcomputing for finding this new feature and for pointing this out!

I also added a bit of documentation to point to an existing mechanism around providing a docker/pythonpath_dev/superset_config_docker.py file, which existed in the logic but wasn't clearly documented. This
solves for my use case where I was working a PR adding features around Slack, where I wanted to provide an API key configuration without risks of commiting/pushing it back to the repo.

closes #28026

As reported here #28026, in
#27953 I added support for a
supplemental `docker/.env-local` override file, not thinking it was
going to be required upon starting docker-compose. I thought I had
tested that while switching branches but somehow didn't test that
docker-compose would simply boot up in the absence of that file.

Since this is confusing to most, and we don't really have a script
I can latch onto to `touch docker/.env-local` for now, i decided
to backtrack and simply  remove the convenience "feature".
People should find an alternate way to alter their environment
somehow before starting the service.

I added a bit of documentation to point to an existing mechanism
around providing a `docker/pythonpath_dev/superset_config_docker.py`
file, which existed in the logic but wasn't clearly documented. This
solves for my use case where I was working a PR adding features
around Slack, where I wanted to provide an API key configuration
without risks of commiting/pushing it back to the repo.
@github-actions github-actions Bot added doc Namespace | Anything related to documentation preset-io labels Apr 15, 2024
@mistercrunch mistercrunch mentioned this pull request Apr 15, 2024
3 tasks
@pull-request-size pull-request-size Bot added size/L and removed size/M labels Apr 15, 2024
@mistercrunch mistercrunch changed the title fix: backtracking on adding docker/.env-local for docker-compose fix: support docker/.env-local for docker-compose Apr 15, 2024
Comment thread .gitignore
messages.mo

docker/requirements-local.txt
docker/.env-local
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

turns out that's covered by docker/*local* bellow

@rusackas rusackas requested a review from dpgaspar April 15, 2024 17:11
@rusackas
Copy link
Copy Markdown
Member

@artofcomputing please feel free to leave a review if you're so inclined :)

Copy link
Copy Markdown
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I like the new syntax/optionality model better :)

@mistercrunch
Copy link
Copy Markdown
Member Author

docker-compose is getting very usable for dev workflows - having it boot fast from my previous PR makes it much more viable too

@mistercrunch mistercrunch merged commit 0c12369 into master Apr 15, 2024
@mistercrunch mistercrunch deleted the backtrack-env-local branch April 15, 2024 17:23
Copy link
Copy Markdown
Contributor

@artofcomputing artofcomputing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, ran locally to test it, environment was up fast 🚀

@mistercrunch
Copy link
Copy Markdown
Member Author

NICE! I'm feeling like CI / docker / docker-compose is getting to a pretty good place.

qleroy pushed a commit to qleroy/superset that referenced this pull request Apr 28, 2024
vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 4.1.0 First shipped in 4.1.0 labels Nov 27, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels doc Namespace | Anything related to documentation preset-io size/L 🚢 4.1.0 First shipped in 4.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docker\.env-local not found

3 participants