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

Command requiring app name should inform when not provided #29

Open
Cryptophobia opened this issue Mar 21, 2018 · 7 comments
Open

Command requiring app name should inform when not provided #29

Cryptophobia opened this issue Mar 21, 2018 · 7 comments

Comments

@Cryptophobia
Copy link
Member

From @vdice on June 7, 2016 22:56

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)

Copied from original issue: deis/workflow-cli#77

@Cryptophobia
Copy link
Member Author

From @bacongobbler on June 8, 2016 4:57

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

@Cryptophobia
Copy link
Member Author

From @helgi on June 8, 2016 4:59

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

@Cryptophobia
Copy link
Member Author

From @bacongobbler on June 8, 2016 5:12

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.

@Cryptophobia
Copy link
Member Author

From @Joshua-Anderson on June 24, 2016 23:29

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

@Cryptophobia
Copy link
Member Author

From @bacongobbler on June 24, 2016 23:44

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. :)

@Cryptophobia
Copy link
Member Author

From @Joshua-Anderson on July 18, 2016 18:54

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

@Cryptophobia
Copy link
Member Author

From @helgi on September 30, 2016 18:35

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?

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

No branches or pull requests

1 participant