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

Improve accounting scripts #95

Merged
merged 28 commits into from
Nov 21, 2023
Merged

Improve accounting scripts #95

merged 28 commits into from
Nov 21, 2023

Conversation

apdibbo
Copy link
Collaborator

@apdibbo apdibbo commented Oct 5, 2023

Convert to Python3

Use dedicated accounting database user

Improvements to the accounting suite

Add accounting for other components

Copy link
Collaborator

@DavidFair DavidFair left a comment

Choose a reason for hiding this comment

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

Partial review

OpenStack-accounting/readme.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@anish-mudaraddi anish-mudaraddi left a comment

Choose a reason for hiding this comment

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

I've done a first-pass - I've raised some suggestions - these will apply to all scripts - I've just chosen a few to comment on

You should also add unit-tests to these scripts - though we can leave this for another PR 😄

@apdibbo
Copy link
Collaborator Author

apdibbo commented Oct 5, 2023

@anish-mudaraddi @meoflynn @DavidFair I believe I've addressed your feedback on this.

@apdibbo apdibbo changed the title [Draft] Improve accounting scripts Improve accounting scripts Oct 23, 2023
@apdibbo
Copy link
Collaborator Author

apdibbo commented Oct 23, 2023

I think this is ready now

Copy link
Contributor

@meoflynn meoflynn left a comment

Choose a reason for hiding this comment

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

Looks good to me, I just had a question about the glance accounting query

This procedure generates accounting data for cinder
*/
SELECT
IFNULL(v.availability_zone, 'nova') AS AvailabilityZone,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to double check, here is it only selecting nova as the availability zone if there isn't an availability zone for a given volume (e.g. volume doesn't have a ceph availability zone and is just null)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that is what that's doing

OpenStack-accounting/sql/glance_get_accounting_data.sql Outdated Show resolved Hide resolved
meoflynn
meoflynn previously approved these changes Oct 23, 2023
DavidFair
DavidFair previously approved these changes Oct 31, 2023
@apdibbo apdibbo dismissed stale reviews from DavidFair and meoflynn via d0be537 November 7, 2023 12:06
@DavidFair DavidFair merged commit 7708756 into stfc:master Nov 21, 2023
1 check passed
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