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

8932 - fix pushing to Docker Hub #9236

Merged
merged 1 commit into from
Dec 19, 2022
Merged

Conversation

poikilotherm
Copy link
Contributor

What this PR does / why we need it:

The Docker Login Actions do not enable logging in to docker.io, but a standard registry. By not overriding the registry, Docker Maven Plugin will use the one supplied by Docker context.

This is basically a typo and was tested with https://github.com/poikilotherm/dataverse/tree/test-8933 to identify the causing issue.

Which issue(s) this PR closes:

Closes #8932

Special notes for your reviewer:
This is a "typo" and took some debugging. Not sure this needs deeper reviewing.

Suggestions on how to test this:
Was tested with a feature branch, as mentioned above. The failing action could be reproduced to fail in the feature branch action and the ONLY difference is the additional option.
Not sure what else should be tested.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Nope.

Is there a release notes update needed for this change?:
Nope, this is a type, the main PR had a release note.

Additional documentation:
None.

The Docker Login Actions do not enable logging in to
docker.io, but a standard registry. By not overriding
the registry, Docker Maven Plugin will use the
one supplied by Docker context (which the authorization
was done for by docker/login-action)
@poikilotherm poikilotherm added the Component: Containers Anything related to cloudy Dataverse, shipped in containers. label Dec 18, 2022
@poikilotherm
Copy link
Contributor Author

@mreekie @pdurbin this is VERY small and a fix for a slipped through bug. I very much believe it will not be necessary to give this more attention than 5 minutes to simply merge it, as there is not very much to review nor test nor QA.

I will nonetheless give this a "Size: 3" label. I would be glad if you folks could "just merge" to make the larger idea of #8932 a success story. Thank you!

@poikilotherm poikilotherm added the Size: 3 A percentage of a sprint. 2.1 hours. label Dec 18, 2022
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Looks fine and won't impact the main project at all.

@pdurbin pdurbin merged commit 880ec33 into IQSS:develop Dec 19, 2022
@pdurbin pdurbin added this to the 5.13 milestone Dec 19, 2022
@pdurbin pdurbin removed the Size: 3 A percentage of a sprint. 2.1 hours. label Dec 19, 2022
@poikilotherm poikilotherm deleted the 8932-fix-push branch May 21, 2023 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Containers Anything related to cloudy Dataverse, shipped in containers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request/Idea: create a base container image providing a Dataverse-tuned Payara application server
2 participants