-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-29261: Clean up GitHub Actions for Docker Release #6127
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
base: master
Are you sure you want to change the base?
Conversation
default: '0.10.2' | ||
|
||
env: | ||
DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @simhadri-g
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's also present in build.yml: HIVE-28303: Develocity integration (#5279)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used by - https://develocity.apache.org/scans?search.rootProjectNames=Hive
ASF doc : https://infra.apache.org/gradle.html
But i am not sure if anyone is actually using the dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh-huh. It is likely useful. Let me restore it back.
5f91fa9
- name: Prepare common environment variables | ||
run: | | ||
echo "namespace=${{ vars.DOCKER_NAMESPACE || 'apache' }}" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I parameterized this part so that a contributor can test the action on their fork.
mvn clean package -DskipTests -Pdist | ||
ls ./packaging/target/ | ||
mv ./packaging/target/apache-hive-*-bin.tar.gz ./packaging/src/docker/ | ||
rm -rf ./packaging/target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just because I encountered a disk full. We might have a smarter way though this is enough.
https://github.com/okumin/hive/actions/runs/18424087084
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm +1
have you tried to trigger updated workflow?
platforms: linux/amd64,linux/arm64 | ||
push: true | ||
tags: apache/hive:${{ env.tag }} | ||
tags: ${{ env.namespace }}/hive:${{ env.tag }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check this,we i implemented this last time ${{ env.namespace }}
ended up returning the wrong value like apachejenkins
or something like that and the docker image was pushed to wrong repo. As a result, i had to hard code it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you elaborate on that issue? When each contributor tests it on their own repo, when the configuration is wrong, pushing an image would fail with an authorization error like this. In case that a release manager builds it in apache/hive, the apache repo is correctly used.
|
What changes were proposed in this pull request?
This PR would unify
buildFromArchive
andbuildFromSource
, which have a large common part.https://issues.apache.org/jira/browse/HIVE-29261
Why are the changes needed?
I plan to add smoke tests(HIVE-29260) for Docker images and add Helm support. Before adding those additional steps, I'd like to clean up the current GitHub actions. Otherwise, we always have to replicate new testing or some steps.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
I tested the action with my personal fork and Docker Hub account.
Manual trigger(buildFromArchive)
Releaes Tag(buildFromSource)
Diff between buildFromArchive and buildFromSource