Skip to content

Prevent error when workers config not found, throw warning instead#6568

Merged
mergify[bot] merged 1 commit intoos-autoinst:masterfrom
Wabri:feat/181409-settings
Jul 8, 2025
Merged

Prevent error when workers config not found, throw warning instead#6568
mergify[bot] merged 1 commit intoos-autoinst:masterfrom
Wabri:feat/181409-settings

Conversation

@Wabri
Copy link
Member

@Wabri Wabri commented Jul 4, 2025

Related: https://progress.opensuse.org/issues/181409

This is still a work in progress, the purpose of this is to replace the error with a warning since the instance even without the definition of the configuration is working.

@Wabri Wabri self-assigned this Jul 4, 2025
@Wabri Wabri added the not-ready label Jul 4, 2025
@Wabri Wabri changed the title Prevent error when workers config not found, throw warning instead WIP Prevent error when workers config not found, throw warning instead Jul 4, 2025
@Wabri Wabri force-pushed the feat/181409-settings branch from 071f330 to 8274528 Compare July 4, 2025 16:28
@Wabri Wabri force-pushed the feat/181409-settings branch from 8274528 to c4f0918 Compare July 7, 2025 09:13
@Wabri Wabri changed the title WIP Prevent error when workers config not found, throw warning instead Prevent error when workers config not found, throw warning instead Jul 7, 2025
@Wabri Wabri removed the not-ready label Jul 7, 2025
@Wabri Wabri force-pushed the feat/181409-settings branch from c4f0918 to 10d3ad3 Compare July 7, 2025 09:19
@Martchus
Copy link
Contributor

Martchus commented Jul 7, 2025

I'm afraid you'll need to restructure the corresponding test:

    #   Failed test 'error logged'
    #   at t/24-worker-settings.t line 130.
    #     Structures begin differing at:
    #          $got->[0] = Does not exist
    #     $expected->[0] = 'No config file found.'
    # []
    # Looks like you failed 1 test of 2.

@Martchus
Copy link
Contributor

Martchus commented Jul 7, 2025

And how does this actually look like as a whole (together with the other startup log messages)? I'm just wondering because you're effectively moving the place where we log this and therefore possibly change the order of startup log messages.

@Wabri Wabri force-pushed the feat/181409-settings branch from 10d3ad3 to 7391c52 Compare July 7, 2025 15:33
@codecov
Copy link

codecov bot commented Jul 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@2d67834). Learn more about missing BASE report.
Report is 20 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #6568   +/-   ##
=========================================
  Coverage          ?   99.10%           
=========================================
  Files             ?      399           
  Lines             ?    40630           
  Branches          ?        0           
=========================================
  Hits              ?    40265           
  Misses            ?      365           
  Partials          ?        0           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Wabri
Copy link
Member Author

Wabri commented Jul 8, 2025

@Martchus finally find the way to spin a single-instance container with local changes and this is the result:

+ su _openqa-worker -c '/opt/openqa/script/worker --instance 1'
+ [[ -n '' ]]
+ wait
[INFO] workers.ini not found, using default settings
[info] worker 1:
 - config file:
 - name used to register:            1e039ebbb480
 - worker address (WORKER_HOSTNAME): localhost
 - isotovideo version:               44
 - websocket API version:            1
 - web UI hosts:                     localhost
 - class:                            ?
 - no cleanup:                       no
 - pool directory:                   /var/lib/openqa/pool/1
[info] Project dir for host localhost is /var/lib/openqa/share
[info] Registering with openQA localhost

The log info is shown as expected!

I'm preparing a new pr with a new docker file to spin the single instance container with local changes.

@Martchus
Copy link
Contributor

Martchus commented Jul 8, 2025

The inconsistent capitalization of the info messages is not nice but I guess it is acceptable. (I guess I would have made it so it would still log everything were it was logged before and just make the parameter passing between the function calls a bit more involved. I guess your simple approach is ok, too.)

@mergify mergify bot merged commit a9d64b7 into os-autoinst:master Jul 8, 2025
51 checks passed
@Wabri Wabri deleted the feat/181409-settings branch July 9, 2025 08:36
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