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

docs: document rpc api versioning process. #2901

Closed
wants to merge 3 commits into from

Conversation

dnldd
Copy link
Member

@dnldd dnldd commented Mar 16, 2022

This documents the process expected to be followed by contributors in versioning JSON-RPC API endpoints.

work towards #2746.

@matthawkins90
Copy link
Contributor

matthawkins90 commented Mar 17, 2022

I think this update is awesome. Had a quick question.

From the previous issue, the two main approaches are:

  1. A single versioned endpoint for the entire RPC API, or
  2. Individual versioned endpoints for each RPC method.

I just want to clarify what you mean by "Per-RPC Versioning" and "versioned per-RPC endpoint" When you say those words, are you referring to the 2nd approach above, or just that the RPC API will have versions in general?

(The reason I ask is because I also couldn't tell from the previous issue which approach you were going to move forward with. But the pro/con writeup was definitely helpful)

@dnldd
Copy link
Member Author

dnldd commented Mar 17, 2022

After discussing the two approaches in chat with davec, versioning each RPC individually was the better approach and that is what this versioning process recommends. "Per-RPC Versioning" and "versioned per-RPC endpoint" are all referring to individual versioned endpoint for each RPC method here. Open to suggestions on wording it better. I will update the text to clearly indicate this where it unclear. I was supposed to comment on the issue regarding the preferred approach before writing the docs, missed a step there.

This documents the process expected to be followed by
contributors in versioning json-rpc api endpoints.
Copy link
Contributor

@matthawkins90 matthawkins90 left a comment

Choose a reason for hiding this comment

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

Looking really good, and I like this schema.
Hopefully my comments are clear and helpful!


### 1. Per-RPC Versioning

To streamline API access in the process of transitioning between release
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the word streamline, but I think this sentence could be worded a bit better. I stumbled over it a few times. I also tweaked some of the language about "versioning endpoints" to be as clear as possible, I hope. See what you think.

"To prevent API compatibility issues when when transitioning between release versions, dcrd's JSON-RPC API methods will each have an individually versioned endpoint. Per-method endpoint versioning provides the flexibility needed to isolate the development of each method, and allows each method to evolve individually and exclusively from each other."

My thought process is:

  • RPC is a protocol (adjective)
  • The API is the general category (noun)
  • The method is the main "thing" we're talking about.
  • The endpoint is the way to access the main thing
  • The endpoints are what are changing, but they're the method endpoints.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think that's clearer. Will make the changes.

docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
@dnldd
Copy link
Member Author

dnldd commented Mar 18, 2022

I'm going to leave this in draft still and look to add a section for versioning websocket endpoints.

Copy link
Contributor

@matthawkins90 matthawkins90 left a comment

Choose a reason for hiding this comment

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

I think the comments and structure makes sense. Unfortunately, I'm only a novice with golang, so I can't really comment on the code examples.

docs/rpc_api_versioning.md Outdated Show resolved Hide resolved
// ...
}
```

With the outlined schemes adhered to, the versioned endpoint should be ready
Copy link
Contributor

Choose a reason for hiding this comment

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

This last sentence is clunky, and I'm not sure what you're trying to say. Is this a list of remaining steps to take before the new versioned endpoints API can be active? I don't really understand the second half of the sentence, but I gave it my best shot. Please feel free to modify:

Once all dcrd JSON-RPC API methods are updated using the schema outlined above, the versioned endpoints will be ready for use. However, dcrctl and other consumers will need to update their rpc/jsonrpc/types dependencies and logic.

Copy link
Member Author

@dnldd dnldd Mar 24, 2022

Choose a reason for hiding this comment

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

I'm saying dcrctl needs to be rebuilt with the updated jsonrpc types dependency to be able to access the new versioned commands.

@dnldd
Copy link
Member Author

dnldd commented Mar 24, 2022

Just realized having the default method name resolve to the current version of the endpoint will break consumers once they update. We don't want that, should be resolving to the older version instead. Will make the needed changes soon.

This adds a section for versioning websockets according to the per-method
versioning scheme. This also updates the documentation to keep the default
method name resolving to the older version of a versioned endpoint to avoid
breaking consumers on an upgrade.
@dnldd dnldd force-pushed the document_rpc_api_versioning branch from 10524df to 510259f Compare March 24, 2022 23:09
@dnldd dnldd marked this pull request as ready for review March 24, 2022 23:13
@davecgh davecgh added the documentation Issues and/or pull requests related to documentation. label Apr 13, 2022
@davecgh davecgh closed this Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues and/or pull requests related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants