Skip to content

Add a 'metadata' field to the proto definition for a usage update#9

Merged
ianmcorvidae merged 2 commits into
cyverse-de:mainfrom
ianmcorvidae:qms-update-meta
Jun 13, 2025
Merged

Add a 'metadata' field to the proto definition for a usage update#9
ianmcorvidae merged 2 commits into
cyverse-de:mainfrom
ianmcorvidae:qms-update-meta

Conversation

@ianmcorvidae

Copy link
Copy Markdown
Member

QMS itself doesn't use these protos I don't think (at least not from what I could see) but stuff calling into it will, like resource-usage-api. This should, hopefully, be the right change to match with cyverse/qms#67

@ianmcorvidae ianmcorvidae requested a review from johnworth June 3, 2025 16:49

@slr71 slr71 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The field itself looks good. I think that code needs to be generated before the changes can be checked in. You can find some instructions for doing that in the README.

@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedgolang/​github.com/​cyverse-de/​p@​v0.0.0-20250418234247-219994b829fb ⏵ v0.0.0-20250419020147-88501eaa978e10010010010050 -50

View full report

@ianmcorvidae

Copy link
Copy Markdown
Member Author

Ah, yes, sorry. I did my best at that -- I ended up with a newer java protobuf version in the process, as well as a newer protoc, so hopefully that's fine. If needed I can probably downgrade or someone else can regenerate the relevant code, but hopefully it's not a big deal, anyway.

@slr71

slr71 commented Jun 12, 2025

Copy link
Copy Markdown
Member

Ah, that makes sense. Sounds good. I can see if I can regenerate the code and see what happens. 🤞

@slr71

slr71 commented Jun 12, 2025

Copy link
Copy Markdown
Member

Ah, I commented before I realized that you generated the code. I think we can merge it and deal with any fallout that occurs when we update downstream repos. 👍

@slr71 slr71 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! :shipit:

@ianmcorvidae

Copy link
Copy Markdown
Member Author

Alright, let's see what I break 🔨

Thanks for the review!

@ianmcorvidae ianmcorvidae merged commit 282887a into cyverse-de:main Jun 13, 2025
2 checks passed
@ianmcorvidae ianmcorvidae deleted the qms-update-meta branch June 13, 2025 17:36
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.

2 participants