You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/02/10 23:40:58 UTC
[GitHub] [incubator-superset] suddjian opened a new pull request #9114: fix
Dockerfile for frontend builds
suddjian opened a new pull request #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114
### CATEGORY
Choose one
- [x] Bug Fix
- [ ] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
My previous PR introduced a bug to the Dockerfile. This fixes it.
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
I've verified this by running `docker-compose up --build`, but again would appreciate verification from someone more well-versed in docker-ology
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: Fixes #9112
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
@mistercrunch @nytai
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on issue #9114: fix
Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
suddjian commented on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584429361
I think you're right, thanks for catching that @nytai
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request
#9114: fix Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#discussion_r378113798
##########
File path: Dockerfile
##########
@@ -81,6 +82,7 @@ RUN useradd --user-group --no-create-home --no-log-init --shell /bin/bash supers
COPY --from=superset-py /usr/local/lib/python3.6/site-packages/ /usr/local/lib/python3.6/site-packages/
# Copying site-packages doesn't move the CLIs, so let's copy them one by one
COPY --from=superset-py /usr/local/bin/gunicorn /usr/local/bin/celery /usr/local/bin/flask /usr/bin/
+COPY --from=superset-node /app/superset/static/assets /app/superset/static/assets
Review comment:
Have not tested it yet, but seems like this should be:
`COPY --from=superset-node /app/superset/assets /app/superset/static/assets`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request
#9114: fix Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#discussion_r377877480
##########
File path: Dockerfile
##########
@@ -81,6 +82,7 @@ RUN useradd --user-group --no-create-home --no-log-init --shell /bin/bash supers
COPY --from=superset-py /usr/local/lib/python3.6/site-packages/ /usr/local/lib/python3.6/site-packages/
# Copying site-packages doesn't move the CLIs, so let's copy them one by one
COPY --from=superset-py /usr/local/bin/gunicorn /usr/local/bin/celery /usr/local/bin/flask /usr/bin/
+COPY --from=superset-node /app/superset/assets /app/superset/assets
Review comment:
shouldn't these be `/superset/static/assets/` ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #9114:
fix Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584415532
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=h1) Report
> Merging [#9114](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/291306392443a5a0d0e2ee0cc4a95d37c56d4589?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9114 +/- ##
=========================================
- Coverage 59.1% 59.1% -0.01%
=========================================
Files 372 372
Lines 11920 11922 +2
Branches 2917 2919 +2
=========================================
+ Hits 7045 7046 +1
- Misses 4693 4694 +1
Partials 182 182
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/chart/chartAction.js](https://codecov.io/gh/apache/incubator-superset/pull/9114/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L2NoYXJ0QWN0aW9uLmpz) | `43.33% <0%> (+0.09%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=footer). Last update [2913063...9bc622a](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian edited a comment on issue #9114: fix
Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
suddjian edited a comment on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584458879
Whew. Had some trials and tribulations as you can see from the commit log but I dare say this is ready to go. Approvals/criticisms welcome.
One change I was forced to make was moving `version_info.json` from `/static/assets` to `/static`. Issues arose around it being built by python code while webpack owns the contents of `/assets`. version_info isn't really an asset anyway, so I would argue that `/static` is the correct location for it in this directory structure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on issue #9114: fix
Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
suddjian commented on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584458879
Whew. Had some trials and tribulations as you can see from the commit log but I dare say this is ready to go. Approvals/criticisms welcome.
One change I was forced to make was moving `version_info.json` from `/static/assets` to `/static`, since it is built by python code and webpack cleans the contents of `/assets`. version_info isn't really an asset anyway, so I would argue that `/static` is the correct location for it in this directory structure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #9114:
fix Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584415532
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=h1) Report
> Merging [#9114](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/291306392443a5a0d0e2ee0cc4a95d37c56d4589?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9114 +/- ##
======================================
Coverage 59.1% 59.1%
======================================
Files 372 372
Lines 11920 11920
Branches 2917 2917
======================================
Hits 7045 7045
Misses 4693 4693
Partials 182 182
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=footer). Last update [2913063...65dfcae](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9114: fix
Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584415532
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=h1) Report
> Merging [#9114](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/291306392443a5a0d0e2ee0cc4a95d37c56d4589?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9114 +/- ##
======================================
Coverage 59.1% 59.1%
======================================
Files 372 372
Lines 11920 11920
Branches 2917 2917
======================================
Hits 7045 7045
Misses 4693 4693
Partials 182 182
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=footer). Last update [2913063...d1d0f05](https://codecov.io/gh/apache/incubator-superset/pull/9114?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian edited a comment on issue #9114: fix
Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
suddjian edited a comment on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584458879
Whew. Had some trials and tribulations as you can see from the commit log but I dare say this is ready to go. Approvals/criticisms welcome.
One change I was forced to make was moving `version_info.json` from `/static/assets` to `/static`. Issues arose around it being built by python code while webpack owns the contents of `/assets`. version_info isn't really an asset anyway, so I would argue that `/static` is a more correct location for it in this directory structure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on issue #9114: fix Dockerfile
for frontend builds
Posted by GitBox <gi...@apache.org>.
nytai commented on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584953512
should also fix https://github.com/apache/incubator-superset/issues/9108, https://github.com/apache/incubator-superset/issues/9116
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on issue #9114: fix Dockerfile
for frontend builds
Posted by GitBox <gi...@apache.org>.
nytai commented on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584429222
Looks like `version_info.json` is produced by running setup.py. I think it's only meant to end up in the actual pip tarball package under `superset/static/assets/version_info.json`. Looking at `setup.py` I'm not sure the paths are correct for generating `version_info.json`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian edited a comment on issue #9114: fix
Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
suddjian edited a comment on issue #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#issuecomment-584458879
Whew. Had some trials and tribulations as you can see from the commit log but I dare say this is ready to go. Approvals/criticisms welcome.
One change I was forced to make was moving `version_info.json` from `/static/assets` to `/static`. Issues arose around it being built by python code and also around webpack cleaning the contents of `/assets`. version_info isn't really an asset anyway, so I would argue that `/static` is the correct location for it in this directory structure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar merged pull request #9114: [docker]
fix, Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
dpgaspar merged pull request #9114: [docker] fix, Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request
#9114: fix Dockerfile for frontend builds
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #9114: fix Dockerfile for frontend builds
URL: https://github.com/apache/incubator-superset/pull/9114#discussion_r377876856
##########
File path: docker-compose.yml
##########
@@ -26,6 +26,7 @@ x-superset-volumes: &superset-volumes
- ./docker/docker-init.sh:/app/docker-init.sh
- ./docker/pythonpath_dev:/app/pythonpath
- ./superset:/app/superset
+ - ./superset-frontend:/app/superset-frontend
Review comment:
👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org