-
Notifications
You must be signed in to change notification settings - Fork 213
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
Making docker user names consistent #5238
Making docker user names consistent #5238
Conversation
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'm starting with the review of catalog/Dockerfile
.
In this file there are a few references to /home/airflow/...
. This the path to the $HOME
directory when the username is airflow
. When the username is changed to ov_user
, the path to the $HOME
directory will become /home/ov_user/...
.
Once you make this change, the CI + CD workflow will be able to proceed further and we'll see if the other checks are passing.
Co-authored-by: Dhruv Bhanushali <[email protected]>
Co-authored-by: Dhruv Bhanushali <[email protected]>
@dhruvkb What exactly is a DC_USER? Do we need to change it as well? |
@tejaswarathe I'm sorry about not checking this earlier but it seems that the Can you try reverting the changes to the Airflow code? The CI + CD workflow will be able to run normally then. |
Reverted the name changes for the catalog, completed the change for API, ingestion_server, indexer_worker. |
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.
Thanks for the contribution @tejaswarathe. LGTM, this is a good normalisation of Docker usernames.
I'll also ask for one more review from another maintainer because it's touching multiple different services but with CI passing, I'm fairly confident that everything is good to go.
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.
Looks good to me! Thank you, @tejaswarathe 💯
Fixes
Fixes #1145 by @dhruvkb
Description
This PR changes the docker usernames for services using different names to a single, meaningful name 'ov_user'.
Changes to the airflow user name have been intentionally left out because the airflow image has to use the airflow user name.
Testing Instructions
Checklist
Update index.md
).main
) or a parent feature branch.ov just catalog/generate-docs
for catalogPRs) or the media properties generator (
ov just catalog/generate-docs media-props
for the catalog or
ov just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin