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.

Sign up for GitHub

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

Jump to bottom

Avoid 500 response and error in logs when filename param not specified #4539

Merged
merged 1 commit into from Mar 2, 2022
Merged

Avoid 500 response and error in logs when filename param not specified #4539

merged 1 commit into from Mar 2, 2022

Conversation

Martchus
Copy link
Contributor

@Martchus Martchus commented Mar 1, 2022

okurz
okurz reviewed Mar 1, 2022
View reviewed changes
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message is a bit too long, rest is fine

@Martchus
Avoid 500 response and error log when filename param not specified
57b3cf8 
* This is about the routes for serving the video/logfile pages.
* Prevent log message "Could not render a response …" and 500 response
* See https://progress.opensuse.org/issues/107719
Martchus added a commit to Martchus/openqa-logwarn that referenced this pull request Mar 2, 2022
@Martchus
Revert "Add test for Could not render a response at …"
876559a 
This reverts commit 8571e2e.
and 3002196 as they the issue
has been fixed by os-autoinst/openQA#4539.
@Martchus Martchus mentioned this pull request Mar 2, 2022
Merged
@codecov
Copy link

codecov bot commented Mar 2, 2022 •
edited

Codecov Report

Merging #4539 (57b3cf8) into master (7a50bad) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4539   +/-   ##
=======================================
  Coverage   97.96%   97.96%           
=======================================
  Files         374      374           
  Lines       34167    34171    +4     
=======================================
+ Hits        33472    33476    +4     
  Misses        695      695
Impacted Files Coverage Δ
lib/OpenQA/WebAPI.pm 96.82% <100.00%> (ø)
lib/OpenQA/WebAPI/Plugin/Helpers.pm 99.19% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a50bad...57b3cf8. Read the comment docs.

@mergify mergify bot merged commit a0ac6ca into os-autoinst:master Mar 2, 2022
@kalikiana
Copy link
Member

Commit message is a bit too long, rest is fine

Looks fine to me.

Checking commit messages against "^.{1,72}(\n|$)"...
- OK: "Avoid 500 response and error log when `filename` param not specified

@Martchus Martchus deleted the response branch March 2, 2022 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@okurz okurz okurz left review comments

@perlpunk perlpunk perlpunk approved these changes

@kalikiana kalikiana kalikiana approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Martchus @kalikiana @perlpunk @okurz