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

Remove namespace list method. #94

Closed
jthomas opened this issue Jan 10, 2018 · 8 comments
Closed

Remove namespace list method. #94

jthomas opened this issue Jan 10, 2018 · 8 comments

Comments

@jthomas
Copy link
Member

jthomas commented Jan 10, 2018

API endpoint is being removed as per the discussion in:
apache/openwhisk#3153

This should be replicated in the client library and published as a new major version.

@starpit
Copy link
Contributor

starpit commented Jan 10, 2018

can we sugar this in the npm? rather than having all clients sugar it?

@rabbah
Copy link
Member

rabbah commented Jan 10, 2018

promise.all(actions.list(), packages.list(), ...)?

@starpit
Copy link
Contributor

starpit commented Jan 10, 2018

almost, each of the component promises needs to be .catch'd. i don't think we want the whole thing to fail fast if any one list call fails?

@rabbah
Copy link
Member

rabbah commented Jan 10, 2018

To a first approximation, id think so - in case of error for one of the lists would you return an empty list in the catch?

@starpit
Copy link
Contributor

starpit commented Jan 10, 2018

this sounds good. do we need a side-band to report partial success? how does the backend API behave in this respect? if it fails fast, then we could just mimic that.

@jthomas
Copy link
Member Author

jthomas commented Jan 11, 2018

Do people use this API endpoint regularly today? Is it used through the SDK in some of the project tools?

The client library has been kept as a thin wrapper over API until now. I was favouring removing this API, as the platform has, rather than shimming for this reason and as it didn't seem to have an obvious usage for most developers.

If we do use this a lot in other projects, that makes sense to add the shim proposed in the PR. Otherwise, I'd vote to remove it.

@starpit
Copy link
Contributor

starpit commented Jan 11, 2018

we did get a feature request for it in the shell. i was mostly hoping that we could break the backend, while breaking as few clients as possible. we could control this, by shipping an updated npm with the updated backend, etc.

i'd be ok marking the npm API as @deprecated

@jthomas
Copy link
Member Author

jthomas commented Jan 11, 2018

@starpit If you have usage in shell, let's merge the PR above. It's simple enough.

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

No branches or pull requests

3 participants