Skip to content

Use volumeID, not attachmentID, to detach volume#2939

Merged
pierreprinetti merged 1 commit intogophercloud:masterfrom
stephenfin:issue-2861
Feb 26, 2024
Merged

Use volumeID, not attachmentID, to detach volume#2939
pierreprinetti merged 1 commit intogophercloud:masterfrom
stephenfin:issue-2861

Conversation

@stephenfin
Copy link
Copy Markdown
Contributor

@stephenfin stephenfin commented Feb 21, 2024

Closes: #2861

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

coveralls commented Feb 21, 2024 •
edited
Loading

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
Copy Markdown
Contributor

EmilienM commented Feb 21, 2024

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
Copy Markdown
Contributor Author

stephenfin commented Feb 22, 2024

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
Copy Markdown
Contributor Author

stephenfin commented Feb 26, 2024

CI failures look unrelated

@pierreprinetti
Copy link
Copy Markdown
Member

pierreprinetti commented Feb 26, 2024

@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
Copy Markdown
Contributor Author

stephenfin commented Feb 26, 2024

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
Copy Markdown
Member

pierreprinetti commented Feb 26, 2024

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
@gophercloud-backport-bot gophercloud-backport-bot bot mentioned this pull request Feb 26, 2024
Merged
@pierreprinetti
Copy link
Copy Markdown
Member

pierreprinetti commented Feb 26, 2024

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