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

Proper tracking of production proto files #12003

Open
aaronc opened this issue May 19, 2022 · 11 comments
Open

Proper tracking of production proto files #12003

aaronc opened this issue May 19, 2022 · 11 comments
Assignees

Comments

@aaronc
Copy link
Member

aaronc commented May 19, 2022

I first addressed this issue in #10582 (comment) and commented briefly on a solution here: #11993 (comment).

Problem Definition

Currently, the main branch has a mixture of proto files which are used in production and still in development. This isn't simply a question of whether the proto package is v1beta1, v1, etc. because some v1beta1 packages are used in production and should be considered stable in spite of containing beta. Also, some v1 packages are still in flux because they're not yet used in production.

Thus what is in the api module and buf schema registry is not reliable for use against production chains.

Proposed Solution

I propose create a branch release/api where proto files that are used to generate code in a non-alpha/beta/rc release are backported to. This release/api should be the 100% certain source of truth for which proto files are actually used in production and should generally be updated right after a non-alpha/beta/rc release is cut.

When these proto files are updated, two things should happen:

  • the api module code should get regenerated and tagged with a (non-alpha/beta/rc) release
  • the proto files should get pushed to buf schema registry

In this way, the latest commit on the buf schema registry and the latest release tag of the api module will always contain a set of production proto files which can be used to target any production SDK release.

Also, we should used release/api as the input to buf breaking and make this a required check so that we know we are never breaking production proto files.

In the meantime, this will allow iteration of non-production proto files on main and other branches without confusing clients. Whenever a tag of the api module is needed in a pre-release of another module, we can make an alpha or beta tag of api on main.

@alexanderbez
Copy link
Contributor

Going to ask some silly questions first...

... because some v1beta1 packages are used in production and should be considered stable in spite of containing beta

So why are they called *beta* then and not have beta removed?

Also, some v1 packages are still in flux because they're not yet used in production.

So why are they called v1 then if they're not stable?

Also, to look at what's in production, why not just look at the latest tag(s) released?

@alexanderbez
Copy link
Contributor

So re-reading this, is the idea that release/api only used for canonical proto schemas and that's it? If so, I think this makes sense.

@aaronc
Copy link
Member Author

aaronc commented May 20, 2022

Going to ask some silly questions first...

... because some v1beta1 packages are used in production and should be considered stable in spite of containing beta

So why are they called *beta* then and not have beta removed?

This will break clients which depend on v1beta1 being in the type URL. Just removing that will mean cosmjs, keplr, etc. will all get broken. Maybe something we can do eventually, but probably not worth it right now.

Also, some v1 packages are still in flux because they're not yet used in production.

So why are they called v1 then if they're not stable?

When we are in the process of building a module we may do this on main:

  • have 1 PR which introduces proto files as v1 (because people decide that's what we want to call it)
  • work on the implementation in another PR
  • find that the proto files need to change a bit so the things that are in v1 can change still because this hasn't been released

The alternative is that we can always do alpha or beta until right at the point where we cut a release but this involves more complicated refactoring late in the game. And also, because of the multi-module approach, alpha and beta proto files from other modules that are WIP can leak into the release tag of a stable module

Also, to look at what's in production, why not just look at the latest tag(s) released?

That's fine if we only have one SDK release tag, but if we have sub-modules then this could combine a mixture of things. For instance, maybe we create the release branch for staking when gov still has some WIP stuff in its proto files.

@aaronc
Copy link
Member Author

aaronc commented May 20, 2022

So re-reading this, is the idea that release/api only used for canonical proto schemas and that's it? If so, I think this makes sense.

Exactly. Just the canonical proto files + api module that are in production releases.

@alexanderbez
Copy link
Contributor

Makes sense to me then.

@aaronc
Copy link
Member Author

aaronc commented May 20, 2022

It's kind of a nasty problem honestly. This is the best solution I've been able to think of. If someone has a better solution I'm all ears

@aaronc
Copy link
Member Author

aaronc commented May 20, 2022

Actually the alternative I can think of is having per go module a separate:

  • proto folder
  • buf schema registry module
  • api go module

So you'd have:

  • x/bank/proto - proto files and buf module
  • x/bank/api - go module

And no root /proto or /api folders

So there's a one to one mapping of go modules to proto and api modules. This is probably more correct, but seems like a fair amount of reorganization

@alexanderbez
Copy link
Contributor

Would rather go with the ideal approach if it's possible after 0.46

@robert-zaremba
Copy link
Collaborator

robert-zaremba commented May 20, 2022

I think the sub-sub module (#12003 (comment)) is more clear, because it is well encapsulated and can be tagged together with the module release.

If I understand it correctly, we will still have v1, v2 subdirectories, right?

  • in the sub-sub-module version: x/bank/proto/v1beta1 and in the future x/bank/proto/v1, same for x/bank/api/* - and both tagged as x/bank/api-v1...
  • in the original idea: release/api/bank/v1beta1 and release/api/bank/v1 only for proto files -- and all tagged, together with other modules (release/api/authz ...) as release/api-v1...

@aaronc
Copy link
Member Author

aaronc commented May 20, 2022

It would actually be x/bank/proto/cosmos/bank/v1beta1, etc.

@aaronc
Copy link
Member Author

aaronc commented May 20, 2022

We could structure api go modules in two different ways:

  1. cosmossdk.io/api/cosmos/bank - so we keep the same structure as now but just separate go modules
  2. cosmossdk.io/x/bank/api - but then the imports would by default be cosmossdk.io/x/bank/api/cosmos/bank/v1beta1 which is a bit verbose...

I think I prefer approach 1 which wouldn't change any existing package imports using the api module

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

No branches or pull requests

5 participants