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

don't crash when a route is 404 #129

Merged
merged 1 commit into from
Jul 13, 2016

Conversation

chrono
Copy link
Contributor

@chrono chrono commented Jul 13, 2016

multiple issues:

  • resolve() might throw, handle that case
  • unhandled exception in process_request leads to last successful
    request_model remain in DataCollector and being used to create
    a response model for the current failed request, leading to
    a constraint violation thrown by the db (creating another response
    for the previously succesful request)

resolution

  • handle 404 by saving NULL/None as view name
  • clean out DataCollector state when starting a request, before doing
    anything.

multiple issues:
 * resolve() might throw, handle that case
 * unhandled exception in `process_request` leads to last successful
   `request_model` remain in `DataCollector` and being used to create
   a response model for the current failed request, leading to
   a constraint violation thrown by the db (creating another response
   for the previously succesful request)

resolution
 * handle 404 by saving NULL/None as view name
 * clean out `DataCollector` state when starting a request, before doing
   anything.
@avelis
Copy link
Collaborator

avelis commented Jul 13, 2016

@chrono This might indirectly help #26

@avelis avelis merged commit 4355083 into jazzband:master Jul 13, 2016
@avelis
Copy link
Collaborator

avelis commented Jul 13, 2016

@chrono Thanks for the contribution and for addressing this.

@chrono chrono deleted the 404-crash-prevention branch July 13, 2016 21:47
@lpolech
Copy link

lpolech commented Jul 26, 2016

Hi, when is it planned to deploy this fix as a new version of silk?

@avelis
Copy link
Collaborator

avelis commented Jul 26, 2016

@Tosterr I will try to get to by EOD today.

@lpolech
Copy link

lpolech commented Jul 28, 2016

@avelis thanks for the response, since there is no new version I would like to ask if it is possible to do it before Friday EOB?

@avelis
Copy link
Collaborator

avelis commented Jul 28, 2016

@Tosterr I think I can do that. Sorry for the delays on release.

@avelis
Copy link
Collaborator

avelis commented Jul 28, 2016

@Tosterr I published 0.6.2 to PyPi. I also made a release for it on Github for reference.

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

Successfully merging this pull request may close these issues.

3 participants