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

Use needles from correct ref of NEEDLES_DIR rebased version #6097

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Wabri
Copy link
Member

@Wabri Wabri commented Jan 3, 2025

Related to #5175

I've rebased the branch with the current master, so now is possible to merge and continue without conflicts

sdclarke and others added 6 commits January 3, 2025 15:52
If the NEEDLES_DIR (or CASEDIR as a backup) variable is a git repo URL
with a fragment specifying the ref of the repository, we attempt to use
it as the source of needles when viewing needle diffs in test results.

If the NEEDLES_GIT_SHA variable is set, we use that instead as it should
be the most accurate way to determine exactly which commit was used
There is now a minion task which will clean up alternate needle files
created by the WebUI. The minimum retention time of the needle files can
be set in the config, and defaults to 30 minutes
* Improve error handling
* Simplify code
* Split code into smaller functions
* Move needle-related code into its own module so utilities don't become
  too big
* Avoid using a shell to run Git commands; this breaks when needle paths
  contain characters that needed escaping
* Avoid invoking Git commands if temporary needle files are still present
* Use the usual directory the web UI stores cache files in (instead of
  hard-coding `/tmp/…`)
* Output temporary files directly instead of using intermediate buffer
* Keep using the real needle path as before for populating last seen/match
  because using the relative path breaks compatibility with existing
  databases and this change is supposedly not required anyway
* See https://progress.opensuse.org/issues/154783
* Use the settings key `temp_needle_refs_retention` which makes it clear
  that this setting is about temporary needle refs
* Document settings key in default `openqa.ini`
* Adapt the cleanup to be in accordance with the changed path for temporary
  needle refs
* Rename cleanup task to `limit_temp_needle_refs` to be more in-line with
  existing cleanup tasks
* Add systemd units to enqueue the cleanup task automatically
* Change the default retention to two hours (from 30 minutes)
* Use the modification time instead of the access time because the access
  time might be updated very to easily unintendedly
* Use signal guard to retry cleanup in case the gru service is restarted
* See https://progress.opensuse.org/issues/154783
Copy link

codecov bot commented Jan 3, 2025

Codecov Report

Attention: Patch coverage is 70.13889% with 43 lines in your changes missing coverage. Please review.

Project coverage is 98.88%. Comparing base (bb83b72) to head (a605475).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
lib/OpenQA/WebAPI/Controller/Step.pm 52.63% 18 Missing ⚠️
lib/OpenQA/Needles.pm 64.86% 13 Missing ⚠️
lib/OpenQA/Git.pm 10.00% 9 Missing ⚠️
lib/OpenQA/WebAPI/Controller/File.pm 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6097      +/-   ##
==========================================
- Coverage   98.98%   98.88%   -0.11%     
==========================================
  Files         396      398       +2     
  Lines       39549    39672     +123     
==========================================
+ Hits        39149    39229      +80     
- Misses        400      443      +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Wabri Wabri marked this pull request as draft January 3, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants