Visitar URL original
Use slices for tag qs filters by stephenfin · Pull Request #3918 · gophercloud/gophercloud · GitHub
Skip to content

Use slices for tag qs filters - #3918

Open
stephenfin wants to merge 7 commits into
gophercloud:mainfrom
shiftstack:tags-qs-slice
Open

Use slices for tag qs filters#3918
stephenfin wants to merge 7 commits into
gophercloud:mainfrom
shiftstack:tags-qs-slice

Conversation

@stephenfin

@stephenfin stephenfin commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

This was already done for the loadbalancer service. Do it for other services.

This is a breaking change that should not be backported to v2. While here, we also fix a TODO with another filter.

@github-actions github-actions Bot added edit:networking This PR updates networking code edit:compute This PR updates compute code edit:identity This PR updates identity code semver:major Breaking change labels Aug 7, 2026
@coveralls

coveralls commented Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Coverage Status

coverage: 63.947%. remained the same — shiftstack:tags-qs-slice into gophercloud:main

IlanZuckerman
IlanZuckerman reviewed Aug 9, 2026
View reviewed changes

@IlanZuckerman IlanZuckerman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would add a few UT's that would prove the query string is built correctly.
For example assert that list opts turn into ?tags=a,b (not ?tags=a&tags=b).

Comment thread openstack/networking/v2/extensions/trunks/requests.go Resolved
@stephenfin
stephenfin marked this pull request as draft August 10, 2026 10:42
@stephenfin
stephenfin marked this pull request as ready for review August 14, 2026 11:06
@stephenfin

stephenfin commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

The networking failures are unrelated. #3945 will address them.

@winiciusallan

winiciusallan commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Now we have #3945 merged, I believe we can re-run the jobs. @stephenfin Would you mind triggering them?

@stephenfin

stephenfin commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Now we have #3945 merged, I believe we can re-run the jobs. @stephenfin Would you mind triggering them?

I've rebased and forced pushed

winiciusallan
winiciusallan reviewed Aug 21, 2026
View reviewed changes

@winiciusallan winiciusallan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found another set of fields that could be potentially changed to use a slice, see

https://github.com/gophercloud/gophercloud/blob/main/openstack/orchestration/v1/stacks/requests.go#L257-L267

while we're here and if it makes sense, i believe it's worth changing.

Comment thread openstack/compute/v2/servers/requests.go Resolved
@stephenfin
networking: Use slice for tag filters
4d0a53a 
This is more intuitive.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@stephenfin
identity: Use slice for tag filters
3d8cc6c 
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@stephenfin
compute: Use slice for tag filters
4402164 
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@stephenfin
compute: Test server tag filters in acceptance tests
2b5abf6 
Cover matching and non-matching results for all four tag filters using\nmulti-element slices.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Assisted-by: OpenAI gpt-6.1-sol (via pi)
@stephenfin
orchestration: Use slice for tag filters
29de781 
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@stephenfin
orchestration: Test stack tag filters in acceptance tests
219fe0f 
Cover matching and non-matching results for all four tag filters using multi-element slices.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Assisted-by: OpenAI gpt-6.1-sol (via pi)
@stephenfin
networking: Resolve TODO
294c275 
Change ListOpts.RevisionNumber to a *int.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@stephenfin

stephenfin commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

I found another set of fields that could be potentially changed to use a slice, see

main/openstack/orchestration/v1/stacks/requests.go#L257-L267

while we're here and if it makes sense, i believe it's worth changing.

Not sure how I missed those. Fixed now in any case.

winiciusallan
winiciusallan approved these changes Oct 27, 2026
View reviewed changes

@winiciusallan winiciusallan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! 👌

@github-actions github-actions Bot added the edit:orchestration This PR updates orchestration code label Oct 27, 2026
@mandre

mandre commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

this would need a rebase to kick the required CI jobs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mandre mandre mandre approved these changes
@winiciusallan winiciusallan winiciusallan approved these changes
+1 more reviewer
@IlanZuckerman IlanZuckerman IlanZuckerman left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

edit:compute This PR updates compute code edit:identity This PR updates identity code edit:networking This PR updates networking code edit:orchestration This PR updates orchestration code semver:major Breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@stephenfin @coveralls @winiciusallan @mandre @IlanZuckerman