Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

replace single filter with multiple filters #1853

Merged
merged 3 commits into from Dec 16, 2015
Merged

Conversation

yifan-gu
Copy link
Contributor

@yifan-gu yifan-gu commented Dec 9, 2015

This is a break change for API service:
1, replaces the single filter field with multiple filters in ListPods and ListImages.
2, filters are combined with OR, and conditions within a filter are combined with AND, which provides a better flexibility.

cc @dgonyeo

@yifan-gu yifan-gu force-pushed the api_filter branch 2 times, most recently from 33a9ae1 to 7461247 Compare December 9, 2015 02:05
@yifan-gu
Copy link
Contributor Author

yifan-gu commented Dec 9, 2015

maybe I just go and implement a simple stringset instead.


package set

type String map[string]struct{}
Copy link
Member

Choose a reason for hiding this comment

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

Imagine if we had generics...

@cgonyeo
Copy link
Member

cgonyeo commented Dec 10, 2015

Looks like some of the tests may have gotten tripped up by the switch to ANDing together all of the fields in a given filter:

=== RUN TestAPIServiceListInspectPods
--- FAIL: TestAPIServiceListInspectPods (3.77s)
    rkt_api_service_test.go:37: Running rkt install
    rkt_api_service_test.go:42: Running rkt api service
    rkt_tests.go:111: Running command: /home/runner/rkt/builds/build-rkt-coreos-/build-rkt-0.12.0+git/bin/rkt --dir=/tmp/datadir-790932015 --local-config=/tmp/localdir-059865026 --system-config=/tmp/systemdir-518606905 --insecure-options=image run /home/runner/rkt/builds/build-rkt-coreos-/build-rkt-0.12.0+git/tmp/functional/test-tmp/rkt-inspect-print.aci
    rkt_api_service_test.go:238: Unexpected result: [], should see non-zero pods
=== RUN TestAPIServiceListInspectImages
--- FAIL: TestAPIServiceListInspectImages (0.36s)
    rkt_api_service_test.go:37: Running rkt install
    rkt_api_service_test.go:42: Running rkt api service
    rkt_api_service_test.go:280: Unexpected result: [], should see non-zero images

Other than that, LGTM.

@yifan-gu
Copy link
Contributor Author

@dgonyeo thanks, will fix the tests :)

@yifan-gu
Copy link
Contributor Author

Will need a rebase after #1786

@yifan-gu yifan-gu force-pushed the api_filter branch 2 times, most recently from aaddb5a to 96d78df Compare December 14, 2015 19:50
@yifan-gu
Copy link
Contributor Author

Rebased.

@yifan-gu
Copy link
Contributor Author

ping @dgonyeo ?

@cgonyeo
Copy link
Member

cgonyeo commented Dec 16, 2015

LGTM

yifan-gu added a commit that referenced this pull request Dec 16, 2015
replace single filter with multiple filters
@yifan-gu yifan-gu merged commit ddfa976 into rkt:master Dec 16, 2015
@yifan-gu yifan-gu deleted the api_filter branch December 16, 2015 22:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants