-
Notifications
You must be signed in to change notification settings - Fork 239
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
[Runtimes] Fix and align with_source_archive()
#1831
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.
Great test coverage
mlrun/runtimes/function.py
Outdated
def get_secrets(keys): | ||
secrets = {} | ||
if ( | ||
project | ||
and get_k8s_helper(silent=True).is_running_inside_kubernetes_cluster() | ||
): | ||
secrets = function._get_k8s().get_project_secret_data(project, keys) | ||
for key in keys: | ||
if key in builder_env: | ||
secrets[key] = builder_env.get(key) | ||
return secrets |
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.
No reason for this to be nested, it's making it harder to read the code
You don't really need function._get_k8s()
you can simply get_k8s_helper()
and you can pass it the rest of the variables
mlrun/runtimes/kubejob.py
Outdated
# make sure we disable load_on_run mode if the source code is in the image | ||
build.load_source_on_run = False |
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.
Not sure I follow, if you build an image, why it means that the source code is in the image ?
with_source_archive()
No description provided.