Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(jobs): use new docker image #2945

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

bodinsamuel
Copy link
Collaborator

@bodinsamuel bodinsamuel commented Nov 5, 2024

Describe your changes

  • Jobs Use new docker image
    Already deployed in staging, nb: requires manual change before deployment on prod

@bodinsamuel bodinsamuel marked this pull request as ready for review November 5, 2024 15:06
@bodinsamuel bodinsamuel requested a review from TBonnin November 5, 2024 15:06
Copy link
Collaborator

@TBonnin TBonnin left a comment

Choose a reason for hiding this comment

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

I don't remember why jobs is not yet using the unified image. Do you remember? @bodinsamuel

@@ -91,12 +91,15 @@ RUN true \
&& apt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false

# Do not use root to run the app
USER node
# BUT it does not work with secret mount (could not find a solution yet)
Copy link
Collaborator

Choose a reason for hiding this comment

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

when do we need secret mount?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To load Google credentials for bigquery

Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need to find a solution before merging this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nope, just running as root is good, <i just left the comment to be sure the next person is not trying to put an user back

@bodinsamuel
Copy link
Collaborator Author

I don't remember why jobs is not yet using the unified image.

No big reasons, it was just a matter of taking the time

@bodinsamuel bodinsamuel requested a review from TBonnin November 6, 2024 15:56
@bodinsamuel bodinsamuel enabled auto-merge (squash) November 6, 2024 17:03
@bodinsamuel bodinsamuel merged commit 205cd94 into master Nov 6, 2024
20 checks passed
@bodinsamuel bodinsamuel deleted the sam/24-22-05/fix/jobs-docker-image branch November 6, 2024 17:12
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants