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

Running list of http RPC bugs #1721

Closed
ramfox opened this issue Mar 24, 2021 · 1 comment · Fixed by #1753
Closed

Running list of http RPC bugs #1721

ramfox opened this issue Mar 24, 2021 · 1 comment · Fixed by #1753
Labels
bug code that is not behaving as expected
Milestone

Comments

@ramfox
Copy link
Member

ramfox commented Mar 24, 2021

Known RPC bugs:

Get - rpc doesn't error but no response

Problem:
The http api is expecting a GetResponse over the wire (since that is the return type of the dataset.get method). However, our api handler does further adjustments to the GetResponse depending on what the user requested in their inital request. Eg, if "meta" is selected, only the meta section is returned. There is pagination if the body was requested, etc. So, unlike most of our endpoints, our api response is not the same as our lib response params. When Dispatch tries to decode the response into a GetResponse, it can't because the response isn't shaped like GetResponse.

Potential solution: We may need to unify the possible get responses & figure out how to represent all variations in a GetResponse, and have the lib.Get method return only what is needed, rather than returning the entire dataset and having the api and cmd layers narrow down what needs to be shown.

@ramfox ramfox added the bug code that is not behaving as expected label Mar 24, 2021
@ramfox ramfox added this to the v0.10.0 milestone Mar 24, 2021
@ramfox ramfox mentioned this issue Apr 8, 2021
29 tasks
@ramfox
Copy link
Member Author

ramfox commented Apr 8, 2021

Using this issue to track the work on the endpoints that haven't been refactored to use NewHTTPRequestHandler
All EXCEPT /get, which will be covered elsewhere.

save

  • returns a dataset ref rather than a dataset (that comes back from dispatch)
    • Save should return a *Dataset
  • adjusts the bodypath to be relative rather than absolute
  • returns a "scriptOutput" along side the ref as a message
    OKAY TO NewHTTPRequestHandler-ize

remove

  • attempts to remove from a remote! this brings up more existential questions about source & remote
  • then this can be easily converted to NewHTTPRequestHandler

OKAY TO NewHTTPRequestHandler-ize

PeerList

  • is this even an endpoint we are supporting?
    Should be absorbed into the normal list with a peer filter

Pull
dataset.Pull returns a dataset.Dataset, but the PullHandler returns a reporef.DatasetRef, are we a-okay to just return a dataset.Dataset
should only return dsref.Ref

UnpackHandler

  • this isn't even associated with a lib method, it just reads the body of the request, unzips it, marshals it and sends to back as json. Should this be converted to a lib method? Or should it stay in the api as sugar?
    stays as sugar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug code that is not behaving as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant