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

Strict major minor version checking on v0 and v1 apis #7038

Merged
merged 4 commits into from
Aug 12, 2021

Conversation

ZenGround0
Copy link
Contributor

Related to #7012 #7020 I've been thinking that a step in the right direction is to enforce that new nodes are talking to the same API version both v0 and v1. This won't help the problem of needing to support backwards compatibility for the v0 api for older coder versions but it will prevent nodes from this point onwards from getting too far and erroring in confusing ways when attempting to work against different api versions.

I put this up (1) to see if it breaks CI in interesting ways (2) to get the opinion of @filecoin-project/lotus-maintainers @whyrusleeping and @raulk.

@Stebalien
Copy link
Member

Hm. I think strict version checking makes sense for v1, but not v0. v1 is "unstable", so every version can be breaking. v0 is stable, so new versions should be backwards compatible.

@Stebalien
Copy link
Member

(well, approved once tests pass)

And we can even use even/odd versioning here as well. I.e., when we're ready to "stabilize" v1, we can cut a v2 that's stable and a v3 that's unstable. That should make it easier to track where we need strict version checks and where we don't.

@ZenGround0 ZenGround0 marked this pull request as ready for review August 12, 2021 17:39
@ZenGround0 ZenGround0 requested a review from a team as a code owner August 12, 2021 17:39
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Looks like there is some weirdness with Version on lotus-gateway

@ZenGround0
Copy link
Contributor Author

Yeah the test kit cli testing tool doesn't set the api.RunningNodeType global. Should be fixed now.

@ZenGround0 ZenGround0 merged commit ce58b11 into master Aug 12, 2021
@ZenGround0 ZenGround0 deleted the spike/restrict-api-versions branch August 12, 2021 18:34
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