Enhance Docker #6504

oliviermichaelis · 2018-12-08T12:32:40Z

Refactored deprecated functionalities:
- Used 'celery worker' command instead of 'superset worker' which is
  deprecated since 0.26.0
- Used 'gunicorn' command instead of 'superset runserver'
Changes made to allow a dynamic port configuration of superset at runtime rather
than at buildtime
- ~~Port is now dynamically exposed in docker-compose (default is 8088)~~
- ~~Moved docker healthcheck from Dockerfile to docker-compose~~

codecov-io · 2018-12-08T12:45:13Z

Codecov Report

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

@@           Coverage Diff           @@
##           master    #6504   +/-   ##
=======================================
  Coverage   56.25%   56.25%           
=======================================
  Files         518      518           
  Lines       22982    22982           
  Branches     2744     2744           
=======================================
  Hits        12929    12929           
  Misses       9644     9644           
  Partials      409      409

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 142e7b6...19ca7a1. Read the comment docs.

mistercrunch · 2018-12-08T18:10:39Z

@victornoel do you mind taking a look since you know most above the current docker setup?

Everything Superset-related LGTM

victornoel

@oliviermichaelis basically I think the port change is useless because docker-compose and docker are made so that you don't need to do this. You can just use a fixed port in the container and use docker-compose ports configuration to decide how to expose it. The rest I think @mistercrunch should validate them :)

contrib/docker/.env

contrib/docker/docker-compose.yml

contrib/docker/docker-entrypoint.sh

oliviermichaelis · 2018-12-08T20:44:02Z

As explained by @victornoel my changes to change the port weren't necessary. I'll remove them.

@victornoel what do you think about tagging the superset image in docker-compose.yml with:

image: apache/incubator-superset:latest

victornoel · 2018-12-09T09:15:43Z

As explained by @victornoel my changes to change the port weren't necessary. I'll remove them.

@victornoel what do you think about tagging the superset image in docker-compose.yml with:
image: apache/incubator-superset:latest

Maybe it's not desired, because the image built is a very dev-oriented image with extra stuffs. But If I wanted to tag it, I would use a simple name (because the / separated name implies it's on dockerhub?) and I would use the tag dev, debug or something like that.

Agood question to ask in order to know how to tag is: what would be the usage of such image outside of this docker-compoe?

phoenix24 · 2018-12-11T17:02:50Z

@victornoel as part of the pull-request, do these need to be updated as well?

install/helm/superset/values.yaml
install/helm/superset/templates/deployment.yaml

victornoel · 2018-12-11T17:10:42Z

@phoenix24 very good question, I didn't know those existed :D
I don't think so because the helm chart rely on @amancevice image, not the one used here (which is dedicated to development, not production).

mistercrunch · 2018-12-18T01:01:50Z

LGTM oh my side @victornoel @oliviermichaelis ready for me to merge?

victornoel · 2018-12-18T07:49:40Z

@mistercrunch I don't think @oliviermichaelis applied the changes we discussed (or didn't push them because they are marked as resolved)

oliviermichaelis · 2018-12-19T08:07:43Z

@mistercrunch @victornoel sorry I didn't have time to apply and push the changes yet. I'll try to do that as soon as possible!

kakoni · 2018-12-22T21:00:24Z

So now that we are enhancing, is exposing redis and psql ports a needed feature? (6379:6379 and 5432:5432)

amancevice · 2018-12-22T21:05:55Z

No, I don’t think so. You would want those apps to run in separate containers.

victornoel · 2018-12-22T21:13:24Z

I don't think exposing ports to the host has anything to do with separating services in containers. Currently they are in separate containers and see each other through the private network created by docker-compose. If the ports are exposed to the host, it can be helpful for debugging purpose. This compose file and docker image are here for development, so it makes sense to expose the ports. Le sam. 22 déc. 2018 à 22:06, Alexander Mancevice <notifications@github.com> a écrit :

…

No, I don’t think so. You would want those apps to run in separate containers. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6504 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJ0z5LjqYqQMNh60fXEYyL-WTppzEdJks5u7p7IgaJpZM4ZJndm> .

kakoni · 2018-12-22T21:15:10Z

@victornoel Got it, it does make sense to expose.

amancevice · 2018-12-22T21:36:18Z

Ah, I misunderstood the intent/conflated GitHub issues. My mistake.

Refactored deprecated functionalities: - Used 'celery worker' command instead of 'superset worker' which is deprecated since 0.26.0 - Used 'gunicorn' command instead of 'superset runserver'

oliviermichaelis · 2019-01-03T12:56:25Z

I resolved the merge conflicts with the last commits

victornoel

@mistercrunch seems good to me

oliviermichaelis · 2019-01-04T14:35:30Z

Thanks! :)

Refactored deprecated functionalities: - Used 'celery worker' command instead of 'superset worker' which is deprecated since 0.26.0 - Used 'gunicorn' command instead of 'superset runserver'

* [SIP-3] Scheduled email reports for Slices / Dashboards (#5294) * [scheduled reports] Add support for scheduled reports * Scheduled email reports for slice and dashboard visualization (attachment or inline) * Scheduled email reports for slice data (CSV attachment on inline table) * Each schedule has a list of recipients (all of them can receive a single mail, or separate mails) * All outgoing mails can have a mandatory bcc - for audit purposes. * Each dashboard/slice can have multiple schedules. In addition, this PR also makes a few minor improvements to the celery infrastructure. * Create a common celery app * Added more celery annotations for the tasks * Introduced celery beat * Update docs about concurrency / pools * [scheduled reports] - Debug mode for scheduled emails * [scheduled reports] - Ability to send test mails * [scheduled reports] - Test email functionality - minor improvements * [scheduled reports] - Rebase with master. Minor fixes * [scheduled reports] - Add warning messages * [scheduled reports] - flake8 * [scheduled reports] - fix rebase * [scheduled reports] - fix rebase * [scheduled reports] - fix flake8 * [scheduled reports] Rebase in prep for merge * Fixed alembic tree after rebase * Updated requirements to latest version of packages (and tested) * Removed py2 stuff * [scheduled reports] - fix flake8 * [scheduled reports] - address review comments * [scheduled reports] - rebase with master * Typo: Fixed link (#6087) * Update contributing.md with latest local dev instructions (#6513) * Fix malformed table in docs/visualization.rst (#6409) * Update requests version (#6510) * Update requests version * Run pip-compile --output-file requirements.txt * [warm] Enforcing consistent form-data (#6531) * Avoid resetting margin to 0 (#6536) * Avoid resetting margin to 0 * Fixing margin for pie chart * Refactor teradata to new time_grain_functions spec (#6539) * Refactor teradata to new time_grain_functions spec * Add test for time_grain_functions * Add docker files to gitignore (#6507) * add docker files to gitignore * Update .gitignore * adding in a dependency version to fix an error with Flask CLI (#6547) * Fix string value displaying NaN (#6534) * Documentation Correction to use http.server for Python3 (#6549) * Remove note about snowflake-sqlalchemy stable version regression (#6398) * filter_values documentation fix (#5977) * Pass security manager to QUERY_LOGGER (#6548) * Pass security manager to log_query * Fix lint * Increase size of column `name` in table `ab_view_meu` (#6532) * Create migration script * Use batch operation for ALTER COLUMN * fix addr_str format bug (#6551) * Fix deck.gl Polygon not show (#6545) * Fix deck.gl Polygon not show * Replace "//" to "// " * Secure unsecured views and prevent regressions (#6553) * Secure views and prevent regressions * Force POST on shortner * Fix tests * [cosmetic] add css no-wrap on CRUD's last modified (#6522) * [RfC] Fix URL too long (#6519) When a URL gets too long (usually happens with controls that allow free form text), it creates an array of bugs. * bug when creating a short URL, because the POST request's referrer field is too long * a bug when saving a chart (when the modal shows up) also because of the referrer being too long Some work has been done in the past to mitigate this, and I'm unclear if it's some sort of regression because of the Flask upgrade or some browser change, or whether these bugs have always been around. This is a request for comment on an approach that works. Let me know if you can think of better ideas as to how to manage this. My current solution looks for 8000+ characters URLs and shrinks them to only `/superset/explore/?URL_IS_TOO_LONG_TO_SHARE&form_data=...` and we only keep the formData keys for `datasource` and `viz_type`. Not super elegant but does the trick. * minor, auto tune debug mode when use flask_env (#6550) * minor, auto tune debug mode when use flask_env * update changes * Make owner a m2m relation on datasources (#6544) * Make owner a m2m relation on datasources * Fix pylint * Make migration work in mysql & sqlite * Fix multiple db_migrations heads (#6560) * [SIP-14] remove dependency on yarn in favor of npm (#6541) * [SIP-14] remove dependency on yarn in favor of npm Find more details here #6217 * Add core-js to dev deps * lint * fix cypress * break down cypress_build.sh * fix cypress * Use 'npm ci' in place of 'npm install' * Bump npm req to >=6.5.0 * Enforcing npm@'>=6.5.0' * Using latest lts of nvm * Make cache settings global * remove uneeded 'npm run test' as 'npm run cover' takes care of that * Prefix with 'time' * Trying to upgrade babel-eslint instead of downgrading * upgrading babel-polyfill to '@babel/polyfill' * Moving to babel 7 * remove unused packages * Make boto3/botocore installation optional (#6540) * Make boto3 installation optional * pylinting * [SQL Lab] Allow running multiple statements (#6112) * Allow running multiple statements from SQL Lab * fix tests * More tests * merge heads * fix heads * Adding missing slash (#6567) * [refactor] moving some datasource-related code to the frontend (#5769) * [refactor] moving some datasource-related code to the frontend * fix js tests * fix tests * fix test * json_iso_dttm_ser use for TableViz (#6563) * fix #6530 and add some other chinese translation (#6569) * Fix: updated required cryptography version to 2.4.2 to resolve #6509 (#6516) * Fix: updated required cryptography version to 2.4.2 to resolve #6509 * Fix: updated cryptography version to 2.4.2 to resolve #6509 * Fix: updated cryptography version to 2.4.2 to resolve #6509 * Revert "Fix: updated required cryptography version to 2.4.2 to resolve #6509" This reverts commit b57b08c. * Fix: updated cryptography version to 2.4.2 * Fix multi/dual when no right y axis (#6571) * [design] use angle icons instead of carets for expandable panels (#6564) * [design] use angle icons instead of carets for expandable panels Also moving to the right to conform to the material design specs * Fix cypress test * fixing issue #6572 with Oracle date handling (#6580) * fix Oracle engine specs for dates issue #6572 * fix Oracle engine specs for dates issue #6572 * fix Oracle engine specs for dates issue #6572, removing comment * ng a trailing space * Change margin for slice description in charts for the dashboard view (#6575) Changed margin for the slices description in dashboard view, previously the margin was 5px for all the sides but for improve the view just apply margin for top and bottom. * [bugfix] moving from reactable to reactable-arc fork (#6576) * Using batch_op in db migration 0b1f1ab473c0 (#6581) As I needed to downgrade from db migration 0b1f1ab473c0, I realized I needed to use batch_op against SQLLite. * Finish move to babel 7 (#6573) * Finish move to babel 7 * Bump jest to 23.6.0 * Address comments * Enhance Docker (#6504) Refactored deprecated functionalities: - Used 'celery worker' command instead of 'superset worker' which is deprecated since 0.26.0 - Used 'gunicorn' command instead of 'superset runserver' * add chinese translate (#6592) * Fix bug: some word not translate in js. (#6598) delete the 'null,' in messages.json, so that word can be translate in js. * shift labels down along y-axis (#6596) * Fix 'Uncaught TypeError: Cannot read property 'value' of undefined' #6556 (#6574) closes #6556 * added get_data ; * added cache logic * added cache logic * added cache key logic * add missing part * revised the metric part * added to_dict * fix lint * fix lint * fix lint * fix lint * fix lint * fix flake8 * fix invalid name * fix invalid name * fix invalid name * enable worldcloid * working version * rebase * fixed expore link issue * add endpoint * added migration script WIP * fixed lint * remove migration script * fix flake8 * fix tesT * fix tesT * fix tesT * fix typo * fixed metric format * fixed metric format * fixed metric format * fixed tests * fixed test * fixed druid test * fixed more tests * fixed more tests * fix test for viz * fixed js tests * fixed ts test * fixed ts test * fixed cypress test * fixed the metric format in filterViz

victornoel suggested changes Dec 8, 2018

View reviewed changes

kristw added change:backend Requires changing the backend risk:breaking-change Issues or PRs that will introduce breaking changes labels Dec 10, 2018

kristw added this to Needs review in PRs Status Dec 11, 2018

phoenix24 approved these changes Dec 18, 2018

View reviewed changes

phoenix24 mentioned this pull request Dec 18, 2018

Change port for docker entrypoint and margin for slice description in charts for the dashboard #6397

Closed

oliviermichaelis added 2 commits January 3, 2019 13:13

Enhance Docker

66abe44

Refactored deprecated functionalities: - Used 'celery worker' command instead of 'superset worker' which is deprecated since 0.26.0 - Used 'gunicorn' command instead of 'superset runserver'

Merge branch 'master' into enhance_docker

19ca7a1

victornoel approved these changes Jan 3, 2019

View reviewed changes

mistercrunch merged commit c01230a into apache:master Jan 3, 2019

PRs Status automation moved this from Needs review to Done Jan 3, 2019

mistercrunch added the 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels label Feb 27, 2024

mistercrunch added the 🚢 0.34.0 label Feb 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance Docker #6504

Enhance Docker #6504

oliviermichaelis commented Dec 8, 2018 •

edited

codecov-io commented Dec 8, 2018 •

edited

mistercrunch commented Dec 8, 2018 •

edited

victornoel left a comment

oliviermichaelis commented Dec 8, 2018

victornoel commented Dec 9, 2018

phoenix24 commented Dec 11, 2018

victornoel commented Dec 11, 2018

mistercrunch commented Dec 18, 2018

victornoel commented Dec 18, 2018

oliviermichaelis commented Dec 19, 2018 •

edited

kakoni commented Dec 22, 2018

amancevice commented Dec 22, 2018

victornoel commented Dec 22, 2018 via email

kakoni commented Dec 22, 2018

amancevice commented Dec 22, 2018

oliviermichaelis commented Jan 3, 2019

victornoel left a comment

oliviermichaelis commented Jan 4, 2019

Enhance Docker #6504

Enhance Docker #6504

Conversation

oliviermichaelis commented Dec 8, 2018 • edited

codecov-io commented Dec 8, 2018 • edited

Codecov Report

mistercrunch commented Dec 8, 2018 • edited

victornoel left a comment

Choose a reason for hiding this comment

oliviermichaelis commented Dec 8, 2018

victornoel commented Dec 9, 2018

phoenix24 commented Dec 11, 2018

victornoel commented Dec 11, 2018

mistercrunch commented Dec 18, 2018

victornoel commented Dec 18, 2018

oliviermichaelis commented Dec 19, 2018 • edited

kakoni commented Dec 22, 2018

amancevice commented Dec 22, 2018

victornoel commented Dec 22, 2018 via email

kakoni commented Dec 22, 2018

amancevice commented Dec 22, 2018

oliviermichaelis commented Jan 3, 2019

victornoel left a comment

Choose a reason for hiding this comment

oliviermichaelis commented Jan 4, 2019

oliviermichaelis commented Dec 8, 2018 •

edited

codecov-io commented Dec 8, 2018 •

edited

mistercrunch commented Dec 8, 2018 •

edited

oliviermichaelis commented Dec 19, 2018 •

edited