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/01/05 13:53:45 UTC
[GitHub] [incubator-superset] villebro opened a new pull request #8925: Add
release refinements from 0.35.2 release
villebro opened a new pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925
### CATEGORY
Choose one
- [ ] Bug Fix
- [x] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [x] Build / Development Environment
- [ ] Documentation
### SUMMARY
This PR includes fixes/refinements to help the release process during the `0.35.2` release.
### TEST PLAN
Tested the updated instructions and affected scripts.
----------------------------------------------------------------
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] craig-rueda commented on issue #8925: Add
release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on issue #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#issuecomment-570985997
I agree that "serious" users would likely be cloning and then running `pip install -r ...`, whereas the majority of users are likely just running `pip install apache-superset` which would be the behavior here.
As far as Docker is concerned, we're using the pinned versions of things (via requirements.txt) when building the image that ships to Dockerhub. This Dockerfile just the one that's used for building tarballs :).
----------------------------------------------------------------
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
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363210862
##########
File path: RELEASING/set_release_env.sh
##########
@@ -16,16 +16,27 @@
# limitations under the License.
#
usage() {
- echo "usage: . set_release_env.sh <SUPERSET_VERSION> <SUPERSET_RC> <PGP_KEY_FULLBANE>"
+ echo "usage: . set_release_env.sh <SUPERSET_RC_VERSION> <PGP_KEY_FULLBANE>"
+ echo "example: . set_relese_env.sh 0.35.2rc1 myid@apache.org"
}
-if [ -z "$1" ] || [ -z "$2" ] || [ -z "$3" ]; then
+if [ -z "$1" ] || [ -z "$2" ]; then
usage;
else
- export SUPERSET_VERSION="${1}"
- export SUPERSET_RC="${2}"
- export SUPERSET_PGP_FULLNAME="${3}"
- export SUPERSET_VERSION_RC="${SUPERSET_VERSION}rc${SUPERSET_RC}"
+ if [[ ${1} =~ ^([0-9]+)\.([0-9]+)\.([0-9]+)rc([0-9]+)$ ]]; then
+ VERSION_MAJOR="${BASH_REMATCH[1]}"
+ VERSION_MINOR="${BASH_REMATCH[2]}"
+ VERSION_PATCH="${BASH_REMATCH[3]}"
+ VERSION_RC="${BASH_REMATCH[4]}"
Review comment:
nice!
----------------------------------------------------------------
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] mistercrunch commented on a change in pull
request #8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r365621432
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
One important question is whether a docker image built from certain SHA in the repo be
"as deterministic as can be" or not.
I hate the idea that the same Apache tarball-based docker build would potentially be different, succeed or failed depending on when it is built.
Personally I vote for using the lock files (requirements.txt) for the purpose of building dockers. Make the docker as builds as deterministic as can be.
We do need another mechanism to catch issues around moving builds when doing `pip install apache-superset`. For that I suggest a nightly where we do not use the lock files.
----------------------------------------------------------------
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] mistercrunch commented on issue #8925: Add
release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on issue #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#issuecomment-570937036
A few minor comments, otherwise LGTM.
@dpgaspar @craig-rueda, what do you think regarding using pinned reqs for docker builds?
----------------------------------------------------------------
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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363095249
##########
File path: RELEASING/set_release_env.sh
##########
@@ -16,16 +16,27 @@
# limitations under the License.
#
usage() {
- echo "usage: . set_release_env.sh <SUPERSET_VERSION> <SUPERSET_RC> <PGP_KEY_FULLBANE>"
+ echo "usage: . set_release_env.sh <SUPERSET_RC_VERSION> <PGP_KEY_FULLBANE>"
+ echo "example: . set_relese_env.sh 0.35.2rc1 myid@apache.org"
}
-if [ -z "$1" ] || [ -z "$2" ] || [ -z "$3" ]; then
+if [ -z "$1" ] || [ -z "$2" ]; then
usage;
else
- export SUPERSET_VERSION="${1}"
- export SUPERSET_RC="${2}"
- export SUPERSET_PGP_FULLNAME="${3}"
- export SUPERSET_VERSION_RC="${SUPERSET_VERSION}rc${SUPERSET_RC}"
+ if [[ ${1} =~ ^([0-9]+)\.([0-9]+)\.([0-9]+)rc([0-9]+)$ ]]; then
+ VERSION_MAJOR="${BASH_REMATCH[1]}"
+ VERSION_MINOR="${BASH_REMATCH[2]}"
+ VERSION_PATCH="${BASH_REMATCH[3]}"
+ VERSION_RC="${BASH_REMATCH[4]}"
Review comment:
I felt it more natural to specify the fully qualified RC version and regex that for major/minor/patch/rc.
----------------------------------------------------------------
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
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363208283
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
Not a simple issue. I'm all for pinned dependencies, but @villebro has a point here, since users will be using `pip install apache-superset`. Yet a good deployment should always include a requirements.txt to pin the dependencies. This docker serves has a quick test to catch immediate issues with the release prior to pushing to Apache.
So I think it makes sense not to pin here, it's a plus for us to early catch dependency problems. A nightly build to test possible problems is a plus also.
Finally being too restrictive on `setup.py` can trigger hard to solve issues, for example not allowing user's to bump dependencies that have problems on specific platforms, etc
----------------------------------------------------------------
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] mistercrunch commented on a change in pull
request #8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r365621432
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
One important question is whether a docker image built from certain SHA in the repo be
"as deterministic as can be" or not.
I hate the idea that the same Apache tarball-based docker build would potentially be different, succeed or fail, depending on when it is built.
Personally I vote for using the lock files (requirements.txt) for the purpose of building dockers. Make the docker as builds as deterministic as can be.
We do [eventually] need another mechanism to catch issues around moving builds when doing `pip install .`. For that I suggest a nightly build that does just that, where we do not use the lock files.
----------------------------------------------------------------
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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363113617
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
I think any serious user will do that. However, as long as local instances of Superset are being installed via `pip` without using a prebuilt image, getting the recommended pinned deps will be difficult without doing a wget to the github branch (to my knowledge `pip` does not currently support proper locking). But I haven't looked into this in detail lately, so there may better ways to solve this problem.
----------------------------------------------------------------
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] mistercrunch commented on a change in pull
request #8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363110148
##########
File path: RELEASING/README.md
##########
@@ -69,7 +103,7 @@ set your GITHUB_TOKEN environment variable.
```bash
# will overwrites the local CHANGELOG.md, somehow you need to merge it in
-github-changes -o apache -r incubator-superset --token $GITHUB_TOKEN --between-tags <PREVIOUS_RELEASE_TAG>...<CURRENT_RELEASE_TAG>
+github-changes -o apache -r incubator-superset --token $GITHUB_TOKEN -b $SUPERSET_GITHUB_BRANCH
Review comment:
Had conversations with @dpgaspar before about `github-changes` struggling with cherries, probably because the SHAs are different on different branches... Does `-b` help?
I thought we may need to do something more sophisticated in the future using PR numbers extracted from commit messages...
----------------------------------------------------------------
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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363094859
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
Since `pip` doesn't care what `requirements.txt` has pinned, I felt it more prudent to just let pip pull in whatever a regular user would get when running `pip install apache-superset`
----------------------------------------------------------------
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] mistercrunch commented on a change in pull
request #8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r365621568
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
@craig-rueda thoughts?
----------------------------------------------------------------
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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363094996
##########
File path: RELEASING/README.md
##########
@@ -69,7 +103,7 @@ set your GITHUB_TOKEN environment variable.
```bash
# will overwrites the local CHANGELOG.md, somehow you need to merge it in
-github-changes -o apache -r incubator-superset --token $GITHUB_TOKEN --between-tags <PREVIOUS_RELEASE_TAG>...<CURRENT_RELEASE_TAG>
+github-changes -o apache -r incubator-superset --token $GITHUB_TOKEN -b $SUPERSET_GITHUB_BRANCH
Review comment:
This seems to work much better than the `--between-tags` option.
----------------------------------------------------------------
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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363110956
##########
File path: RELEASING/README.md
##########
@@ -69,7 +103,7 @@ set your GITHUB_TOKEN environment variable.
```bash
# will overwrites the local CHANGELOG.md, somehow you need to merge it in
-github-changes -o apache -r incubator-superset --token $GITHUB_TOKEN --between-tags <PREVIOUS_RELEASE_TAG>...<CURRENT_RELEASE_TAG>
+github-changes -o apache -r incubator-superset --token $GITHUB_TOKEN -b $SUPERSET_GITHUB_BRANCH
Review comment:
It seems `-b` solved all my problems (output completely in line with the log from `git` but with additional github context), so cautiously optimistic about being able to keep using `github-changes`.
----------------------------------------------------------------
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] mistercrunch commented on a change in pull
request #8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363126517
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
We could be more restrictive on the `setup.py` ranges or even pinning deps there, but that makes it hard for people to override in their local environments. But since Superset is not a library but a self-standing app, it's more acceptable to pin deps.
An alternative to catch range issues would be to setup a nightly build that runs a `pip install .` (without using pinned deps in requirements.txt). When that nightly build fails we'd look into the issue and adjust ranges...
----------------------------------------------------------------
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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r367252233
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
To avoid letting this PR get stale, I propose reinstating `pip install -r requirements.txt` here and deferring this discussion to future rounds of release refinements. I'll make the changes (this + a few others I identified yesterday + all issues highlighted by IPMC) soon so we can get this merged as soon as possible.
----------------------------------------------------------------
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
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363210112
##########
File path: RELEASING/README.md
##########
@@ -69,7 +103,7 @@ set your GITHUB_TOKEN environment variable.
```bash
# will overwrites the local CHANGELOG.md, somehow you need to merge it in
-github-changes -o apache -r incubator-superset --token $GITHUB_TOKEN --between-tags <PREVIOUS_RELEASE_TAG>...<CURRENT_RELEASE_TAG>
+github-changes -o apache -r incubator-superset --token $GITHUB_TOKEN -b $SUPERSET_GITHUB_BRANCH
Review comment:
fingers crossed here. Problems may arise on a new minor, 0.36 for example, where we need to diff with master and the SHA's changed
----------------------------------------------------------------
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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363094873
##########
File path: RELEASING/Dockerfile.from_svn_tarball
##########
@@ -57,7 +57,6 @@ RUN cd superset/assets \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
Same here.
----------------------------------------------------------------
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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363110777
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
Valid point. Perhaps default to using `requirements.txt` but add an option `pip` or similar to the script (similar to how `local` works now) to skip this step and pull in whatever a vanilla `pip install` would pull in?
----------------------------------------------------------------
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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363095090
##########
File path: RELEASING/set_release_env.sh
##########
@@ -16,16 +16,27 @@
# limitations under the License.
#
usage() {
- echo "usage: . set_release_env.sh <SUPERSET_VERSION> <SUPERSET_RC> <PGP_KEY_FULLBANE>"
+ echo "usage: . set_release_env.sh <SUPERSET_RC_VERSION> <PGP_KEY_FULLBANE>"
+ echo "example: . set_relese_env.sh 0.35.2rc1 myid@apache.org"
Review comment:
I thought it's ok to have a real version in the example, as the script makes sure it doesn't yet exist before execution.
----------------------------------------------------------------
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] villebro merged pull request #8925: Add
release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925
----------------------------------------------------------------
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] mistercrunch commented on a change in pull
request #8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363111398
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
Can we assume that people would just add docker layers that would force install deps on top or our base image? Maybe I'm missing something but I think it's a better option than parameterizing the Dockerfile.
----------------------------------------------------------------
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] craig-rueda commented on a change in pull
request #8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r365627693
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
Still feel the same in this case, even after the discussion. 😆 For releasing, I think it makes sense to use setup.py, whereas for building docker images that are shipped to docker hub, we should use pinned versions. I think dynamic versions in this case give a better feel for what the end user will see when running 'pip install superset'
----------------------------------------------------------------
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] mistercrunch commented on a change in pull
request #8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r367249669
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
Though here we're not trying to simulate `pip install apache-superset`, the goal is to provide build instructions or a reference dockerfile for building from the signed, official tarball. Ideally everyone building any particular version has the same recipe and gets to the same docker (unless they actively want to alter/deviate).
Side note: at the time where I wrote it there was no official dockerfile in the base of the repo, now that there's one we may be able to just use that one instead (?)
----------------------------------------------------------------
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
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363212695
##########
File path: RELEASING/make_tarball.sh
##########
@@ -22,8 +22,8 @@ usage() {
}
if [ -z "$1" ] || [ -z "$2" ] || [ -z "$3" ]; then
- if [ -z "${SUPERSET_VERSION}" ] || [ -z "${SUPERSET_RC}" ] || [ -z "${SUPERSET_PGP_FULLNAME}" ]; then
- echo "No parameters found an no required environment variables set"
+ if [ -z "${SUPERSET_VERSION}" ] || [ -z "${SUPERSET_RC}" ] || [ -z "${SUPERSET_PGP_FULLNAME}" ] || [ -z "${SUPERSET_RELEASE_RC_TARBALL}" ]; then
+ echo "No parameters found and no required environment variables set"
Review comment:
Since we are using regex to parse superset's version on `set_release_env` it would be nice to propagate this to all the other scripts
----------------------------------------------------------------
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] mistercrunch commented on a change in pull
request #8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363110021
##########
File path: RELEASING/Dockerfile.from_local_tarball
##########
@@ -56,7 +56,6 @@ RUN npm ci \
WORKDIR /home/superset/apache-superset-incubating-$VERSION
RUN pip install --upgrade setuptools pip \
- && pip install -r requirements.txt \
Review comment:
That makes the docker image non deterministic which I think is an issue. Personally I vote for having a very deterministic docker build.
----------------------------------------------------------------
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 #8925: Add
release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#issuecomment-570915361
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8925?src=pr&el=h1) Report
> Merging [#8925](https://codecov.io/gh/apache/incubator-superset/pull/8925?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/478e445a5a52613b5e1b5e7c666946715d7e8506?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8925?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8925 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8925?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/8925?src=pr&el=footer). Last update [478e445...24c71f5](https://codecov.io/gh/apache/incubator-superset/pull/8925?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] villebro commented on a change in pull request
#8925: Add release refinements from 0.35.2 release
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8925: Add release refinements from 0.35.2 release
URL: https://github.com/apache/incubator-superset/pull/8925#discussion_r363094948
##########
File path: RELEASING/README.md
##########
@@ -54,11 +54,45 @@ need to be done at every release.
svn commit -m "Add PGP keys of new Superset committer"
```
+## Setting up the release environment (do every time)
Review comment:
I felt it makes sense to have this section earlier, in part because we now also need the `SUPERSET_GITHUB_BRANCH` env variable for `github-changes`.
----------------------------------------------------------------
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