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): superset permissions and firefox config #14736

Merged
merged 3 commits into from
May 26, 2021

Conversation

dpgaspar
Copy link
Member

@dpgaspar dpgaspar commented May 20, 2021

SUMMARY

This PR fixes/improves the following:

  • Fixes firefox config for thumbnails and alerts & report when running with a low privilege user.
  • Easier way to run dev docker-compose with a low privilege user eg: superset
  • Fixes superset homedir

ADDITIONAL INFORMATION

  • Has associated issue: [Report] Reports not working with OIDC auth #14330
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@codecov
Copy link

codecov bot commented May 20, 2021

Codecov Report

Merging #14736 (379132f) into master (a9d888a) will decrease coverage by 0.23%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14736      +/-   ##
==========================================
- Coverage   77.54%   77.31%   -0.24%     
==========================================
  Files         959      959              
  Lines       48696    48696              
  Branches     5743     5743              
==========================================
- Hits        37762    37647     -115     
- Misses      10733    10848     +115     
  Partials      201      201              
Flag Coverage Δ
hive ?
mysql 81.38% <100.00%> (ø)
postgres 81.40% <100.00%> (ø)
presto ?
python 81.48% <100.00%> (-0.45%) ⬇️
sqlite 81.01% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/config.py 91.09% <100.00%> (ø)
superset/db_engines/hive.py 0.00% <0.00%> (-82.15%) ⬇️
superset/db_engine_specs/hive.py 70.32% <0.00%> (-17.08%) ⬇️
superset/db_engine_specs/presto.py 83.36% <0.00%> (-6.53%) ⬇️
superset/views/database/mixins.py 81.03% <0.00%> (-1.73%) ⬇️
superset/connectors/sqla/models.py 88.46% <0.00%> (-1.69%) ⬇️
superset/db_engine_specs/base.py 87.90% <0.00%> (-0.41%) ⬇️
superset/models/core.py 89.70% <0.00%> (-0.27%) ⬇️
superset/utils/core.py 88.90% <0.00%> (-0.13%) ⬇️

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 a9d888a...379132f. Read the comment docs.

@dpgaspar dpgaspar requested review from villebro and nytai May 20, 2021 12:15
Comment on lines -87 to +88
RUN useradd --user-group --no-create-home --no-log-init --shell /bin/bash superset \
&& mkdir -p ${SUPERSET_HOME} ${PYTHONPATH} \
RUN useradd --user-group -d ${SUPERSET_HOME} --no-log-init --shell /bin/bash superset \
&& mkdir -p ${PYTHONPATH} \
Copy link
Member

Choose a reason for hiding this comment

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

I like this 👍

@@ -41,7 +41,7 @@ if [[ "${1}" == "worker" ]]; then
celery worker --app=superset.tasks.celery_app:app -Ofair -l INFO
elif [[ "${1}" == "beat" ]]; then
echo "Starting Celery beat..."
celery beat --app=superset.tasks.celery_app:app --pidfile /tmp/celerybeat.pid -l INFO
celery beat --app=superset.tasks.celery_app:app --pidfile /tmp/celerybeat.pid -l INFO -s /app/superset_home/celerybeat-schedule
Copy link
Member

Choose a reason for hiding this comment

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

Should we pick the path to /app/superset_home from ${SUPERSET_HOME}?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, changed it

@dpgaspar dpgaspar requested a review from villebro May 25, 2021 09:36
@dpgaspar dpgaspar merged commit d46aa60 into apache:master May 26, 2021
@dpgaspar dpgaspar deleted the fix/docker-user-home branch May 26, 2021 08:12
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
* fix(docker): superset permissions and firefox config

* fix lint

* user SUPERSET_HOME ENV on bootstrap script
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
* fix(docker): superset permissions and firefox config

* fix lint

* user SUPERSET_HOME ENV on bootstrap script
"--high-dpi-support=2.0",
"--headless",
]
WEBDRIVER_OPTION_ARGS = ["--headless", "--marionette"]
Copy link
Member

@john-bodley john-bodley Jun 9, 2022

Choose a reason for hiding this comment

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

@dpgaspar should this just be ["--headless"] given that the WEBDRIVER_TYPE is defaulted to firefox? Or is the comment now wrong.

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
* fix(docker): superset permissions and firefox config

* fix lint

* user SUPERSET_HOME ENV on bootstrap script
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 labels Mar 12, 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 preset-io size/S 🚢 1.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants