-
Notifications
You must be signed in to change notification settings - Fork 174
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
[KFP v2] Fix the S3 secret name is hardcoded in the KFP library. #1030
base: dev
Are you sure you want to change the base?
Conversation
# FIXME: Due to kubeflow/pipelines#10914, secret names cannot be provided as pipeline arguments. | ||
# As a workaround, the secret name is hard coded. | ||
env2key = ComponentUtils.set_secret_key_to_env() | ||
kubernetes.use_secret_as_env(task=execute_job, secret_name="s3-secret", secret_key_to_env=env2key) |
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 suggest defining a constant at the start of the file and using it here. It will simplify for users to update it.
841db06
to
ece26e2
Compare
@revit13 Can you merge with the latest from dev and let me know if this fixes the problem with the kfp1 test for lang_id ? I had to add a read-only secret for hugging face, exposed that in the workflow as an env. variable and then use it in the lang_id_wf.py (more details here: #1068). I am looking at code_quality now. |
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Signed-off-by: Revital Sur <[email protected]>
Why are these changes needed?
FIX - Due to kubeflow/pipelines#10914, we cannot provide secret names as pipeline arguments, so we agreed to a workaround that the secret name will be hard-coded.
The secret names should be in pipeline, so users will be able to overwrite them. Currently it is in the library, so users cannot change it.
Related issue number (if any).
#985