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

feat(grpc): update swagger API to version 1.1 #1106

Merged
merged 4 commits into from Feb 17, 2024

Conversation

b00f
Copy link
Collaborator

@b00f b00f commented Feb 13, 2024

Description

This PR updates the Swagger APIs to version 1.1. In this new version we follow this convention:

  • Each API starts with pactus
  • Each API should follow the same path as gRPC APIs
  • Comments should be on top of the field to generate commented codes.

Copy link

codecov bot commented Feb 13, 2024

Codecov Report

Merging #1106 (a4ea5ce) into main (5f4cddf) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1106   +/-   ##
=======================================
  Coverage   81.40%   81.40%           
=======================================
  Files         173      173           
  Lines        9217     9217           
=======================================
  Hits         7503     7503           
+ Misses       1360     1359    -1     
- Partials      354      355    +1     

Copy link
Contributor

@kehiy kehiy 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. I just have one question. What's the purpose of the pactus prefix instead of v1, and why we removed the v1 prefix? Isn't that helpful when we add the next versions over time?

@b00f
Copy link
Collaborator Author

b00f commented Feb 13, 2024

Isn't that helpful when we add the next versions over time?

I saw that swagger also has version, we bump it to v1.1 in this PR

@kehiy
Copy link
Contributor

kehiy commented Feb 13, 2024

what about SDKs and users that use these endpoints on code? they don't work with Swagger in code and they don't have access to the Swagger version.

@b00f
Copy link
Collaborator Author

b00f commented Feb 13, 2024

Even we add version to API, they still need to update their code. So this version is not really helpful for them

@kehiy
Copy link
Contributor

kehiy commented Feb 13, 2024

what if they want to support different Pactus RPC versions? For example:

PactusSDK => RPCV1, RPCV2 ...

@b00f
Copy link
Collaborator Author

b00f commented Feb 14, 2024

what if they want to support different Pactus RPC versions? For example:

PactusSDK => RPCV1, RPCV2 ...

To keep things simple, we only support one version, and it is the latest one.
However, users can still run Node with an older version and use the old gRPC endpoint.

Imagine in Pactus version 1.2.0 we have RPCV1, and in version 1.3.0 we have RPCV2.
Then, a user can run two nodes: one with version 1.2.0 and another with version 1.3.0 to have both RPCV1 and RPCV2.

@themantre themantre merged commit c709b04 into pactus-project:main Feb 17, 2024
12 checks passed
@b00f b00f deleted the grpc-update-swagger branch March 21, 2024 06: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.

None yet

4 participants