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

add flag to image_name_extractor to return short image name #3622

Merged
merged 2 commits into from Jan 8, 2018

Conversation

hkaj
Copy link
Member

@hkaj hkaj commented Jan 2, 2018

Note: Please remember to review the Datadog Contribution Guidelines
if you have not yet done so.

What does this PR do?

Fix #3082

Motivation

user request

Testing Guidelines

this PR is tied to ... - they should be tested together

Additional Notes

Anything else we should know when reviewing?

Copy link
Contributor

@JulienBalestra JulienBalestra left a comment

Choose a reason for hiding this comment

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

lgtm, just a nit

"""
Returns the image name for a container, either directly from the
container's Image property or by inspecting the image entity if
the reference is its sha256 sum and not its name.
Result is cached for performance, no invalidation planned as image
churn is low on typical hosts.
If short is true, the repository is stripped from the result
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid reading two times the composition of functions, why not doing something like:

image = self.image_name_resolver(co.get('Image', ''))
if short:
    image.split('/')[-1]
return image

@hkaj hkaj merged commit cbebaf6 into master Jan 8, 2018
@hkaj hkaj deleted the haissam/add-short-image-tag branch January 9, 2018 13:03
@truthbk truthbk modified the milestones: 5.22.0, 5.21.0 Jan 10, 2018
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