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.

Sign up for GitHub

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

Jump to bottom

Use volumeID, not attachmentID, to detach volume #2939

Merged
merged 1 commit into from
Feb 26, 2024
Merged

Use volumeID, not attachmentID, to detach volume #2939

merged 1 commit into from
Feb 26, 2024

Conversation

stephenfin
Copy link
Contributor

Closes: #2861

@github-actions github-actions bot added the semver:patch No API change label Feb 21, 2024
@coveralls
Copy link

coveralls commented Feb 21, 2024 •
edited

Coverage Status

coverage: 77.277%. remained the same
when pulling 8602cd4 on stephenfin:issue-2861
into a894551 on gophercloud:master.

EmilienM
EmilienM previously approved these changes Feb 21, 2024
View reviewed changes
@EmilienM
Copy link
Contributor

I'm waiting for CI to merge it. If any other reviewer pass around and CI is green, I think this is a good to go.
Thanks for the renaming, indeed less confusing.

@github-actions github-actions bot removed the semver:patch No API change label Feb 22, 2024
@stephenfin
Copy link
Contributor Author

Can someone change the base branch of this for me? I have it pointed against v2 and it seems I should have used master.

@github-actions github-actions bot added the semver:major Breaking change label Feb 22, 2024
@stephenfin
Copy link
Contributor Author

CI failures look unrelated

@pierreprinetti
Copy link
Contributor

@stephenfin please reopen this PR against master instead of v2

@stephenfin stephenfin changed the base branch from v2 to master February 26, 2024 19:05
@stephenfin
compute: Use volumeID, not attachmentID for volume attachments
8602cd4 
The volume attachments API operates on volumes, not attachments. Correct
the variable name.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Closes: gophercloud#2861
@stephenfin
Copy link
Contributor Author

Can someone change the base branch of this for me? I have it pointed against v2 and it seems I should have used master.

Never mind, I could do this: I just has to click the Edit button 🙈

@github-actions github-actions bot added semver:patch No API change and removed semver:major Breaking change labels Feb 26, 2024
@pierreprinetti pierreprinetti added backport-v1 This PR will be backported to v1 backport-v2 labels Feb 26, 2024
@pierreprinetti
Copy link
Contributor

I am trying

Can someone change the base branch of this for me? I have it pointed against v2 and it seems I should have used master.

Never mind, I could do this: I just has to click the Edit button 🙈

I had no idea one could do that!
Thanks. LGTM

@pierreprinetti pierreprinetti merged commit e460c85 into gophercloud:master Feb 26, 2024
26 of 29 checks passed
@gophercloud-backport-bot gophercloud-backport-bot bot mentioned this pull request Feb 26, 2024
Merged
@pierreprinetti
Copy link
Contributor

I expect the backport to v1 to fail, and:

  1. I might learn where to look for the logs
  2. we should manually backport to v1

@stephenfin stephenfin deleted the issue-2861 branch February 26, 2024 21:17
@pierreprinetti pierreprinetti mentioned this pull request Feb 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pierreprinetti pierreprinetti pierreprinetti approved these changes

@EmilienM EmilienM EmilienM left review comments

Assignees
No one assigned
Labels
backport-v1 This PR will be backported to v1 semver:patch No API change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

volumeattach: Volume detach should use volume ID
4 participants
@stephenfin @coveralls @EmilienM @pierreprinetti