Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Create new ListImagesWithOptions method in v10 #1129

Merged
merged 2 commits into from
Jun 12, 2018
Merged

Conversation

aaron7
Copy link
Contributor

@aaron7 aaron7 commented Jun 8, 2018

We originally added options to ListImages in #1084 however we found out we couldn't make this change because of the RPC method signature.

In this PR we revert the ListImages change and instead create ListImagesWithOptions in v10. For API versions before v10 ListImagesWithOptions will continue to work, with the WC service polyfilling the container fields filter option.

@aaron7 aaron7 requested a review from squaremo June 8, 2018 15:39
@aaron7 aaron7 force-pushed the listimages-v10 branch 2 times, most recently from 4020e18 to 8719f7d Compare June 11, 2018 16:48
Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

Looks good ⭐ Some minor shufflings requested.

func (p *ErrorLoggingServer) ListImagesWithOptions(ctx context.Context, opts v10.ListImagesOptions) (_ []v6.ImageStatus, err error) {
defer func() {
if err != nil {
p.logger.Log("method", "ListImages", "error", err)

This comment was marked as abuse.

"github.com/weaveworks/flux/remote"
)

// RPCClientV10 is the rpc-backed implementation of a server, for

This comment was marked as abuse.

"github.com/weaveworks/flux/api/v6"
"github.com/weaveworks/flux/daemon"

This comment was marked as abuse.

@aaron7 aaron7 force-pushed the listimages-v10 branch 2 times, most recently from 526391d to 64be046 Compare June 12, 2018 13:24
@aaron7 aaron7 requested a review from squaremo June 12, 2018 13:25
api/v10/api.go Outdated
@@ -0,0 +1,25 @@
// This package defines the types for Flux API version 9.

This comment was marked as abuse.

This comment was marked as abuse.

Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

Nice. Chomp that last morsel of copypasta and it's good to go.

@aaron7 aaron7 merged commit 03d1eed into master Jun 12, 2018
@aaron7 aaron7 deleted the listimages-v10 branch June 12, 2018 14:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants