-
Notifications
You must be signed in to change notification settings - Fork 310
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
Assert return value of expected_http_error
#1308
Assert return value of expected_http_error
#1308
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
Thanks you @toslunar! We looked at this in the Jupyter Server meeting this week and we'll follow up soon. cc @matthewwiese |
@Zsailer The four tests fail because of the use of
You can see in the error message for If the try/except block is modified to raise try:
validate_model(payload)
self.event_logger.emit(
schema_id=payload.get("schema_id"),
data=payload.get("data"),
timestamp_override=get_timestamp(payload),
)
self.set_status(204)
self.finish()
except web.HTTPError:
raise
except Exception as e:
raise web.HTTPError(500, str(e)) from e |
Thanks, @matthewwiese! The modification to the try/except block looks right to me. I've added you to our list of collaborators—so you can push directly to this PR/branch. If you or @toslunar can update this PR to get the tests passing, I'm happy to merge. Thanks! |
Looks like the jupyterlab_server tests are failing (link) all with the same error: |
@matthewwiese Thank you for fixing the code! |
@matthewwiese I believe the errors are related to jupyterlab/jupyterlab_server#400. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you both!
expected_http_error
returnsbool | None
to be tested.jupyter_server/tests/utils.py
Lines 24 to 45 in c5dc0f6