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

Questions around /push endpoints #1744

Closed
ramfox opened this issue Apr 6, 2021 · 4 comments · Fixed by #1747
Closed

Questions around /push endpoints #1744

ramfox opened this issue Apr 6, 2021 · 4 comments · Fixed by #1747
Assignees
Labels
discussion Open-ended request for feedback refactor A code change that neither fixes a bug nor adds a feature
Milestone

Comments

@ramfox
Copy link
Member

ramfox commented Apr 6, 2021

Right now, we have /push do 3 things depending on the http method

  1. GET - returns a list of your datasets on the given remote as []VersionInfo
  2. POST - expected behavior, pushes a dataset to a remote
  3. DELETE - removes a dataset from a remote
    Seems like we need to add a few more endpoints to our spec to cover cases 1 & 3 or at least make sure they are covered under other endpoints.

Should we add /remote/list and /remote/remove, or should these be folded into /list and /ds/remove?

@ramfox ramfox added refactor A code change that neither fixes a bug nor adds a feature discussion Open-ended request for feedback labels Apr 6, 2021
@ramfox ramfox added this to the v0.10.0 milestone Apr 6, 2021
@ramfox ramfox self-assigned this Apr 6, 2021
@dustmop
Copy link
Contributor

dustmop commented Apr 6, 2021

Personally I like adding both /remote/list and /remote/remove.

@b5
Copy link
Member

b5 commented Apr 6, 2021

Yeah I agree with @dustmop, and think we should add /remote/list for listing your datasets on a remote, and /remote/remove.

Folding the remove functionality into /ds/remove is tempting, but I'm a fan of how the /remote-prefixed endpoints all talk to the remote subsystem.

@ramfox
Copy link
Member Author

ramfox commented Apr 6, 2021

:chefs kiss:

@ramfox
Copy link
Member Author

ramfox commented Apr 7, 2021

As per our in person (zoom) discussion:
Turns out the functionality of # 1, getting a list of datasets from a remote is NOT what that method was doing. It instead was filtering our own datasets & returning the ones marked 'Public'. Since this is not was is expected of a /remote/list endpoint, we are tabling that endpoint for now until we can implement properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Open-ended request for feedback refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants