Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix docker configurations and docker installation guide. #7827

Merged
merged 5 commits into from
Jul 16, 2019
Merged

Fix docker configurations and docker installation guide. #7827

merged 5 commits into from
Jul 16, 2019

Conversation

ali-bahjati
Copy link
Contributor

CATEGORY

  • Bug Fix

SUMMARY

  • Fix installation guide regarding passing enviromental variable to docker-compose.
  • Add gevent installation command in Dockerfile. (Didn't add it in requirements because it's not really a requirement for the whole project)
  • Add user: root:root to allow superset service to work in development mode. Since access to binded superset directory to local requires root user.
  • Also add localhost to postgres and redis services to reduce unintended security risk.

ADDITIONAL INFORMATION

REVIEWERS

… example using this didn't work because of this (and also as the tooltip says the auto option should be available)
- Fix installation guide regarding passing enviromental variable to docker-compose.
- Add gevent installation command in Dockerfile. (Didn't add it in requirements because it's not really a requirement for the whole project)
- Add user: root:root to allow superset service to work in development mode. Since access to binded superset directory to local requires root user.
- Also add localhost to postgres and redis services to reduce unintended security risk.
- Fix installation guide regarding passing enviromental variable to docker-compose.
- Add gevent installation command in Dockerfile. (Didn't add it in requirements because it's not really a requirement for the whole project)
- Add user: root:root to allow superset service to work in development mode. Since access to binded superset directory to local requires root user.
- Also add localhost to postgres and redis services to reduce unintended security risk.
@codecov-io
Copy link

Codecov Report

Merging #7827 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7827   +/-   ##
=======================================
  Coverage   65.89%   65.89%           
=======================================
  Files         459      459           
  Lines       22023    22023           
  Branches     2418     2418           
=======================================
  Hits        14512    14512           
  Misses       7391     7391           
  Partials      120      120

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d08efd3...43269fc. Read the comment docs.

@ali-bahjati
Copy link
Contributor Author

I found out that my PR wasn't rebased with current master so I added rebase after that. I don't know whether it's acceptable or no. If not, please tell me how to fix it.

Copy link

@pl77 pl77 left a comment

Choose a reason for hiding this comment

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

I agree that the docker installation could probably use a revisit. Thanks for starting this!

contrib/docker/docker-compose.yml Show resolved Hide resolved
contrib/docker/docker-compose.yml Show resolved Hide resolved
contrib/docker/Dockerfile Show resolved Hide resolved
@lrosenman
Copy link

0.33-docker-w2.patch.txt

These are the diffs I'm using to get it working.

I'd appreciate some consideration on how to migrate.

These are against the release--0.33 branch.

@Aylr
Copy link
Contributor

Aylr commented Jul 11, 2019

This is similar to #7744

@ali-bahjati
Copy link
Contributor Author

This is similar to #7744

Only part of it.

@ali-bahjati
Copy link
Contributor Author

Any other comment on this PR?
Since the current Docker instructions doesn't work I think it's better to merge this sooner and apply enhancement ideas later.

Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

LGTM

@mistercrunch mistercrunch merged commit a36c136 into apache:master Jul 16, 2019
alex-mark pushed a commit to alex-mark/incubator-superset that referenced this pull request Jul 29, 2019
* Add Auto option to point radius to make it work as expected since the example using this didn't work because of this (and also as the tooltip says the auto option should be available)

* Remove trailing space

* Fix docker configurations and docker installation guide.
- Fix installation guide regarding passing enviromental variable to docker-compose.
- Add gevent installation command in Dockerfile. (Didn't add it in requirements because it's not really a requirement for the whole project)
- Add user: root:root to allow superset service to work in development mode. Since access to binded superset directory to local requires root user.
- Also add localhost to postgres and redis services to reduce unintended security risk.

* Fix docker configurations and docker installation guide.
- Fix installation guide regarding passing enviromental variable to docker-compose.
- Add gevent installation command in Dockerfile. (Didn't add it in requirements because it's not really a requirement for the whole project)
- Add user: root:root to allow superset service to work in development mode. Since access to binded superset directory to local requires root user.
- Also add localhost to postgres and redis services to reduce unintended security risk.
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 28, 2024
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 size/XS 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting up superset using docker
6 participants