Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
⚠️ Use ironic inventory API instead of calling ironic-inspector #1355
⚠️ Use ironic inventory API instead of calling ironic-inspector #1355
Changes from 3 commits
e03450fee6955579ea7c9df0e91baa31c74d8b09e7File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit uncomfortable with this. Is the goal to merge it with the development commit or wait for gophercloud v2 to be released?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
V2 may take more time to get releases. What's the cause of your discomfort? For better or worse, using git commits is a normal thing in the Go world. The last time I tried to hurry gophercloud folks about a release they went "wtf, just use a git hash".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be landing in v2 only? Do you have any guess when would that be? AFAIK when we need to test commits that has landed in dev branches but not ready for release yet, we use pseudo-versions with git hashes. Use of pseudo version signals module is still under development and not stable. But since go has now stricter rules now to generate pseudo-numbers, it can be used incase we cannot wait for proper release to exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, this is going to v2 only because this work involved breaking changes. I don't know when that happens, we can ask on slack.
I think I'm using what you're describing here. Since the last release before splitting the 1.* branch was 1.5.0, it assumes the new version will 1.5.1-something (which is wrong, but go tell go!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm mainly worried about the upgrade path and the potential for API changes that would impact us. It is probably not an issue but I would not want to just skip over it without thinking it through. So how do we expect the upgrade will look like?
We start with a scenario where a user is running an older version of BMO/Ironic relying on the older API and gophercloud. What would be required to upgrade? My understanding is something like this:
This seems easy enough, but definitely worth a mention in the release notes for BMO.
Now my worry is that there would be some migration steps needed to go from gophercloud v1 to v2 and we end up somewhere in the middle. Do you see any situation where some kind of patch would be needed to support the bump to v2? (I know of projects where bugs have made it impossible to upgrade from e.g. 0.3.1 to 0.4.0. Instead users were required to first upgrade to the latest patch release, e.g. 0.3.4 and from there on to 0.4.0.) For example, could there be a required upgrade path for gophercloud to do v1.7.1 -> v2.0.0 and we miss it because we do not have the v1.7.1 patch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, you seem to be conflating the Ironic upgrade and the Gophercloud upgrade.
Gophercloud is just an HTTP library. 2.0.0 will see some Go API changes, but these are easy to address on any bump. For better or worse, Go does not update anything automatically (we will need to keep an eye on dependabot, of course). This is why I'm not sure what you mean by "we don't have the v1.7.1 patch". Gophercloud has no local state.
Ironic upgrade is a bit different. Historically, we have taken bumping microversions very lightly. Now Steven has raised a valid point that some of our consumers may not always use the latest-and-greatest versions. Fair. We need to start having some sort of upgrade docs per release (which I assume don't exist now).
In any case, what are the suggested action items here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation, that makes sense!
It seems like there is not much reason to worry. We just need to document the Ironic version bump so users notice it.
Perhaps we can bring this up at the community meeting today also to make sure developers are aware of it, in case there are any concerns?