Skip to content

all: IDsFromName to return multiple results #156

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

Merged
merged 1 commit into from
Jul 20, 2021

Conversation

pierreprinetti
Copy link
Member

@pierreprinetti pierreprinetti commented Jul 20, 2021

This patch adds a function in the utils packages that returns all the
IDs of the items with the given name.

This patch leaves the behaviour of the existing IDFromName functions
untouched, while it reduces code duplication.

@pierreprinetti pierreprinetti force-pushed the volume_idsfromname branch 2 times, most recently from 463e6c4 to ee6309e Compare July 20, 2021 09:47
@pierreprinetti
Copy link
Member Author

pierreprinetti commented Jul 20, 2021

I consider this patch ready for review (and possibly, merge). Thank you for your time!

CC @jtopjian

@EmilienM
Copy link
Contributor

very handy!
👍

@kayrus
Copy link
Contributor

kayrus commented Jul 20, 2021

I'd sync the code with other services: compute, image, etc.

@jtopjian
Copy link
Contributor

@pierreprinetti

I agree that this is handy - thank you! If it's not too much trouble, @kayrus makes a good point that we should add this for the other resources for consistency. If you don't have the time or desire to do that, just let me know. I'll merge this and then take care of the other resources.

@pierreprinetti
Copy link
Member Author

I submit to the will of the people! 😁

@pierreprinetti
Copy link
Member Author

pierreprinetti commented Jul 20, 2021

@jtopjian
If you agree, while I am at it, I am tempted by removing the "if name == name", since we already use ListOpts to filter query results by name. WDYT?

@pierreprinetti pierreprinetti changed the title volumes: IDsFromName returns multiple results all: IDsFromName to return multiple results Jul 20, 2021
This patch adds a function in the utils packages that returns all the
IDs of the items with the given name.

This patch leaves the behaviour of the existing `IDFromName` functions
untouched, while it reduces code duplication.
@pierreprinetti
Copy link
Member Author

That should be it. I have only updated the latest version per package.

Note that I have removed if s.Name == name from the loop when there was a ListOpts that filtered by name already.

Please spend a moment checking that everything is alright, because that's a lot of copy-pasta and I need some backup eyes

@jtopjian
Copy link
Contributor

I've tested everything but the Manila stuff, since I don't have access to a Manila environment. Short of that, this all looks good to me - thanks so much.

Copy link
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

LGTM - thank you!

@jtopjian jtopjian merged commit 8a3ad2a into gophercloud:master Jul 20, 2021
@pierreprinetti pierreprinetti deleted the volume_idsfromname branch July 20, 2021 19:56
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.

4 participants