Skip to content
This repository has been archived by the owner on Nov 15, 2021. It is now read-only.

Add documention for API servers and RPC methods #664

Open
wants to merge 37 commits into
base: development
Choose a base branch
from

Conversation

jseagrave21
Copy link
Contributor

@jseagrave21 jseagrave21 commented Oct 13, 2018

What current issue(s) does this address, or what feature is it adding?

  • Adds detailed documentation for API server operation and RPC methods
  • Update basicusage.rst for ExtendedJsonRpcApi
  • Updates formatting and fixes typo

How did you solve this problem?
Trial and Error

How did you make sure your solution works?
make docs

Are there any special changes in the code that we should be aware of?
No

Please check the following, if applicable:

  • Did you add any tests?
  • Did you run make lint?
  • Did you run make test?
  • Are you making a PR to a feature branch or development rather than master?
  • Did you add an entry to CHANGELOG.rst? (if not, please do)

Merge CoZ Development into jseagrave21 Development
- fix typo
- update help display to show `--extended-rpc`
- update formatting
- update formatting, introduce a new header for visibility, and modify wording
- clarify interface methods for neo-python API servers
- Include brief description and usage data
- Include default JSON-RPC command list
- Include extended JSON-RPC command list
- Include POST request examples
- Include GET request example
- update formatting
- Begin adding method examples
for compatibility
- re-add seedlist.png
- add remaining method details
@coveralls
Copy link

coveralls commented Oct 13, 2018

Coverage Status

Coverage remained the same at 83.931% when pulling 78505bd on jseagrave21:docs-ExtendedJsonApi into 1aec5f0 on CityOfZion:development.

Copy link
Member

@ixje ixje left a comment

Choose a reason for hiding this comment

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

A couple of small changes to start with, I did not fully read/review the Api_Server file for the reason mentioned below.

I'd like to hear some opinions from @localhuman and @metachris on what they think about keeping a "shadow copy" of RPC command descriptions. I'm leaning towards referring to the offical NEO CLI documentation for the default RPC methods and only describing the extended ones in this documentation. Any thoughts or arguments for/against?

docs/source/basicusage.rst Show resolved Hide resolved
docs/source/basicusage.rst Outdated Show resolved Hide resolved
docs/source/basicusage.rst Outdated Show resolved Hide resolved
docs/source/basicusage.rst Outdated Show resolved Hide resolved
@jseagrave21
Copy link
Contributor Author

jseagrave21 commented Oct 16, 2018

Originally I was just going to write it for the extended methods but I figured it would be good to write a copy for the default methods also because we do not maintain parody with Neo CLI at this moment and we won't because dumpprivkey will be an extended method. Also, our outputs look different than Neo CLI in some cases like getaccountstate and some of our input or allowable inputs are different than Neo CLI. For example, we can refer to the NEO assets by 'neo' and 'gas' in most cases.

- edit the `help` description for `--extended-rpc`
- change wording for `help` for `--extended-rpc`
- remove redundant `ExtendedJsonRpcApi` methods section and update usage to match new help for `ExtendedJsonRpcApi`
- remove reference to websockets
- eliminate reference to websockets
@ixje
Copy link
Member

ixje commented Oct 20, 2018

dumpprivkey is a method asking for trouble so that not being part of the default JSON RPC API is fine.
The idea is that anyone who coded something against the neo-cli RPC server can seamlessly switch to neo-python's RPC server. That means:

  • neo-python must support the exact same inputs as neo-cli
  • neo-python must return the exact same output as neo-cli

Having some additional features like accepting neo/gas instead of the asset id's does not interfere with the above and is ok. I do think we should limit them, just incase some people develop against neo-python RPC server and then want to switch to neo-cli.

@ixje
Copy link
Member

ixje commented Oct 20, 2018

All good. Just waiting for a response from @localhuman @metachris on the documentation duplication #664 (review)

@localhuman
Copy link
Collaborator

Looks good to me!

@jseagrave21
Copy link
Contributor Author

Now that #634 is merged I am going to add it to the list.

- fix a couple typos
- add `getblockheader` method
for compatibility
Merge CoZ Development into jseagrave21 docs-ExtendedJsonAPi
@jseagrave21
Copy link
Contributor Author

Okay, it should be good to go.

@ixje
Copy link
Member

ixje commented Oct 31, 2018

@localhuman I'm cannot tell in what direction "looks good to me" points.
Where I tagged you and Chris is with my idea:

I'm leaning towards referring to the offical NEO CLI documentation for the default RPC methods and only describing the extended ones in this documentation.

but your response could equally mean that the commits by @jseagrave21 look good and they can go in, which would be the opposite. Can you be explicit in what you mean with "looks good to me"?

@jseagrave21
Copy link
Contributor Author

Since #714 was merged, I will update the example.

@jseagrave21
Copy link
Contributor Author

Okay, @localhuman I think this is ready to go. Will you let me know what you would like to keep from this documentation? Referring to @ixje 's comment:

@localhuman I'm cannot tell in what direction "looks good to me" points.
Where I tagged you and Chris is with my idea:

I'm leaning towards referring to the offical NEO CLI documentation for the default RPC methods and only describing the extended ones in this documentation.

but your response could equally mean that the commits by @jseagrave21 look good and they can go in, which would be the opposite. Can you be explicit in what you mean with "looks good to me"?

- add REST port info
- add REST server documentation
@jseagrave21
Copy link
Contributor Author

I also added REST server documentation because other than reading the code and querying a REST server, I haven't seen any documentation for it.

for compatibility
for compatibility
Merge neo-python development into jseagrave21 docs-ExtendedJsonApi
- revert changes to bring to parody with CityOfZion#719
- add instructions for running custom API plugins
- update documentation for CityOfZion#712 and CityOfZion#719
@jseagrave21
Copy link
Contributor Author

@localhuman @ixje I have updated the documentation for #712 and #719. Please let me know what you think.

@jseagrave21
Copy link
Contributor Author

jseagrave21 commented Dec 24, 2018

@ixje With ExtendedJsonRpcApi transitioned to plugins is this PR still salvageable? What documentation would we like to commit to maintaining?

@ixje
Copy link
Member

ixje commented Dec 24, 2018

I think ExtendedJsonRelated documentation should be moved close to the source. So have a readme for new function and what output it produces together with e.g. the rpc-plugin.

I'm still of the opinion that we should not duplicate documentation of the basic commands. NEO is maintaining it, we try to adhere to the same format, so linking to their docs is easier. Otherwise we're taking up a huge task were I'd rather use that time and energy to focus on improving other parts of neo-python.

@jseagrave21
Copy link
Contributor Author

@ixje would you like to keep the REST documentation or examples of inputs for JsonRpcApi? Perhaps some of the fixes for typos and then just delete the rest? I can pair it down.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants