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.

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

Liquid nova report capacity #628

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

Varsius
Copy link
Contributor

@Varsius Varsius commented Dec 13, 2024

Checklist:

  • If this PR is about a plugin, I tested the plugin against an OpenStack cluster.
  • I updated the documentation to describe the semantical or interface changes I introduced.

@coveralls
Copy link

Coverage Status

coverage: 79.342%. remained the same
when pulling b5e8d08 on liquid-nova-report-capacity
into 8284712 on master.

return fmt.Errorf("while serializing Subcapacity Attributes: %w", err)
}
b.Subcapacities = append(b.Subcapacities, liquid.Subcapacity{
// TODO: What should be the naming here? We list subcapacities for cores, ram and instances on the same split flavor capacity
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like there being multiple subcapacities for different aspects of the same thing. Let's have this as a single subcapacity with Capacity: 0, and all the actual data in attributes shaped like the old subcapacities.


for _, subres := range allSubresources {
var attrs SubresourceAttributes
if err = json.Unmarshal(subres.Attributes, &attrs); err != nil { // TODO: Unmarshalling here + marshalling when building the subresources seems inefficient
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider passing the subresources as type liquid.SubresourceBuilder[A] instead.

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.

3 participants