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

[Builder] Fix function image tagging syntax #860

Merged
merged 2 commits into from Apr 24, 2021

Conversation

ihs-nick
Copy link
Contributor

Resolve issue #828

@ihs-nick
Copy link
Contributor Author

ihs-nick commented Apr 19, 2021

@Hedingber, is there something else I need to do in order to get this approved? Right now this causes many copies of any function images that are built. This means that a container registry could be full of just one function.

Also, do you think that this should be changed as well?

def get_fullname(name, project, tag):

The function is:

def get_fullname(name, project, tag):
    if project:
        name = f"{project}-{name}"
    if tag:
        name = f"{name}-{tag}"
    return name

I suspect it should be f"{name}:{tag}"

Here is another function that builds images:

def enrich_image_url(image_url: str) -> str:

Maybe these should all be in one class for consistency?

@yaronha
Copy link
Collaborator

yaronha commented Apr 20, 2021

@ihs-nick AFAIK k8s names cant have :, but in the image it be the right solution

@Hedingber Hedingber changed the title BUGFIX: fix function tagging syntax [Builder] Fix function image tagging syntax Apr 24, 2021
@Hedingber Hedingber merged commit 0e10900 into mlrun:development Apr 24, 2021
@Hedingber
Copy link
Contributor

Hi @ihs-nick sorry it took me some time to get to this one, wanted to run this code through our more robust system tests to verify it doesn't break anything, thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants