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

clair: Ancestry Builder #712

Merged
merged 6 commits into from
Feb 26, 2019
Merged

clair: Ancestry Builder #712

merged 6 commits into from
Feb 26, 2019

Conversation

KeyboardNerd
Copy link
Contributor

worker is removed, we have Ancestry builder! using builder pattern to construct an Ancestry.

analyzer.go Show resolved Hide resolved
ancestry.go Outdated Show resolved Hide resolved
ancestry.go Outdated Show resolved Hide resolved
ancestry.go Show resolved Hide resolved
database/detector.go Outdated Show resolved Hide resolved
blob.go Outdated Show resolved Hide resolved
blob.go Outdated Show resolved Hide resolved
ancestry.go Show resolved Hide resolved
if name == "" {
// TODO(sidac): we'll use the computed ancestry name in the future.
// During the transition, it still requires the user to use the correct
// ancestry name.
Copy link
Contributor

Choose a reason for hiding this comment

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

How big of a change do you think this will be? Why not just strip it out of the API right now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because when getting the ancestry, it requires a name.
We can either do:
return the computed name to the client
or
the client still gives us the name, but we use an extra table to keep the client given name to the internal ancestry name mapping.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should get rid of names and make the computed name an implementation detail. Just let the customer provide the layer list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OR (which I prefer)
The user always query with a list of layer hashes in the request body. This just requires us to change the API a little bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exactly

analyzer.go Outdated Show resolved Hide resolved
@KeyboardNerd KeyboardNerd merged commit 7608186 into quay:master Feb 26, 2019
@KeyboardNerd KeyboardNerd deleted the builder branch February 26, 2019 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants