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

ci: Push protobufs for release tag events #11993

Closed
wants to merge 2 commits into from

Conversation

pinosu
Copy link
Contributor

@pinosu pinosu commented May 19, 2022

Push protobufs when a new release tag event is triggered.

This will help to keep protobufs in sync with the imported version specified in go.mod .

See this comment for better context understanding.

I have...

  • included tags filter to push event in proto-registry GH action

@pinosu pinosu requested a review from a team as a code owner May 19, 2022 09:18
@pinosu pinosu changed the title Push protobufs for release tag events ci: Push protobufs for release tag events May 19, 2022
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

I can't comment on if this is needed or not. @marbar3778 or @aaronc can you chime in?

@tac0turtle
Copy link
Member

hmm, not sure this is needed as proto files are backwards compatible unless the version number changes. Im fine adding this, but are we sure its needed. For 0.45.1 the proto files may not be present due to us following this process after that release.

@aaronc
Copy link
Member

aaronc commented May 19, 2022

Coordination of proto files with buf is actually a mess right now. We should have a release/api branch which has backports of proto files which are considered production ready (aligned with a non-alpha/beta/rc release of some sdk go module). Only release/api should get pushed to buf and non-alpha/beta/rc tags of api should only happen on this branch. Does that make sense @alexanderbez @marbar3778 ?

Also the approach I see the cosmwasm team trying to take with synchronizing against SDK proto files is not in line with how we're trying to approach this. You should not need a separate set of proto files tagged against 0.45.x to be compatible with 0.45. We do not break proto files that are in production so any tag from buf should theoretically be fine. Now the issue with this approach is that what's in buf is a mix of production-ready and alpha/beta proto files. The release/api branch approach I proposed above would address this.

I'll create an issue to track this. It's been on my radar just haven't gotten around this.

I think we should close this PR as it doesn't really address the underlying issue.

@aaronc
Copy link
Member

aaronc commented May 19, 2022

See #12003

@alpe
Copy link
Contributor

alpe commented May 20, 2022

Some context on the problem that may help to understand our motivation in wasmd:
We were aiming for self containing and reproducible builds for us and client projects. Therefore we copied the protobuf files into the third_party folder as it was done in the sdk before.
With the buf repo the sdk versions and source proto files could not be linked easily anymore. I do understand that the protobuf version should reflect compatibility but it would still be a moving target.

When I understand #12003 correct, then this problem would be addressed. Every sdk release can point to a hash/tag in the buf repo and we can do the same. 👍 Thanks for taking this serious and providing an integration path. 🌷

@aaronc
Copy link
Member

aaronc commented May 20, 2022

Every sdk release can point to a hash/tag in the buf repo and we can do the same.

So this isn't quite accurate based on what we're planning to do. 0.46 is effectively the last planned "SDK" release. See #11899. From then on what's in buf will be effectively the latest tagged release of all the modules. To understand what's valid for a given module version, clients will need to make use on Since comments of fields and messages.

@aaronc
Copy link
Member

aaronc commented May 20, 2022

So if for cosmwasm there's a desire to have proto files that are for an older version of some modules than what is tagged in the SDK, it might be hard to have a build of that exact set of proto files, but hopefully the Since comments make it clear what is compatible and what isn't.

Does that seem workable @alpe ?

@aaronc
Copy link
Member

aaronc commented May 20, 2022

Also @alpe sounds like we might take another approach: #12003 (comment)

@tac0turtle
Copy link
Member

tac0turtle commented Jun 21, 2022

@pinosu sorry for delay in reviewing this pr, we are still working on how to structure things in relation to these releases

@tac0turtle
Copy link
Member

@pinosu ill close this pr for now. We are working on fixing this as part of the framework working group. Thank you for opening the PR and bringing your pain points to us. If you have anymore please ping us!!!

@tac0turtle tac0turtle closed this Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants