workflow-cli icon indicating copy to clipboard operation
workflow-cli copied to clipboard

Command requiring app name should inform when not provided

Open vdice opened this issue 9 years ago • 8 comments

When an app name is not provided for deis commands technically requiring one the current default is to return the raw 404 Not Found response from the k8s api:

$ deis config:list
Error:
404 Not Found
detail: Not found.

It would be nice if the command could inform the user of the need/requirement to pass the app name (in the form of -a app_name) to properly proceed. (Basically, if the command hasn't been invoked with this parameter, send some default message to that effect)

vdice avatar Jun 07 '16 22:06 vdice

indeed. This error comes from the server, so any improvements should be made there.

bacongobbler avatar Jun 08 '16 04:06 bacongobbler

Not sure we can given how we use a built in get object or 404 dealio

helgi avatar Jun 08 '16 04:06 helgi

We could try to overwrite with a new function in the api :)

from django import http, shortcuts

def get_object_or_404(klass, *args, **kwargs):
    try:
        shortcuts.get_object_or_404(klass, args, kwargs)
    except http.Http404 as e:
        raise http.Http404(klass.__name__ + ' ' + e.message)

...or something related to that.

bacongobbler avatar Jun 08 '16 05:06 bacongobbler

We now have a ErrNotFound defined in the CLI, so we can just override that with a custom error message for apps.

Joshua-Anderson avatar Jun 24 '16 23:06 Joshua-Anderson

I think this is a server-side thing. we're using get_object_or_404 for some operations, which will only return a 404 without an explanation. This can be munged up with other 404s since it's just a generic "I could not find the object you were looking for" error. Once we actually return a specific error then we can definitely add a ErrAppNotFound in the CLI. :)

bacongobbler avatar Jun 24 '16 23:06 bacongobbler

I opened an issue on the controller for this: deis/controller#833

Joshua-Anderson avatar Jul 18 '16 18:07 Joshua-Anderson

DRF is masking this https://github.com/tomchristie/django-rest-framework/blob/b82ec540ad447dcf703a9b21e9e94976109fd175/rest_framework/views.py#L81-L83 where Django does try to bubble up better information https://github.com/django/django/blob/master/django/shortcuts.py#L92-L93

The question becomes what should we do? We could update our exception handler to provide more context than just "Not Found" which is the standard 404 text at this point.

Could incorporate @bacongobbler idea into the exception handler and call it a day?

helgi avatar Sep 30 '16 18:09 helgi

This issue was moved to teamhephy/workflow-cli#29

Cryptophobia avatar Mar 21 '18 14:03 Cryptophobia