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

Try a backup copy if the downloaded cutechess-cli is not working. #1369

Merged
merged 1 commit into from Jul 21, 2022

Conversation

vdbergh
Copy link
Contributor

@vdbergh vdbergh commented Jun 25, 2022

Logic:

  1. Check if cutechess-cli exists and is working.

  2. If not: download it.

  3. Check if downloaded cutechess-cli is working.

  4. If not: try to restore a backup copy.

  5. If successful check if the backup copy is working.

  6. If not: bail out.

Also: do not treat MacOS specially in the updater.

Also: only setup/verify during worker startup.

Solves #1368 .

@vdbergh
Copy link
Contributor Author

vdbergh commented Jul 16, 2022

@ppigazzini I wonder if you could have a look at this PR? I think it is a nice refactoring of the setup of cutechess-cli. Perhaps it might also deal correctly with this issue (if it is solvable at all):

https://tests.stockfishchess.org/actions?action=&user=technologov&before=1657958686.643&count=100

worker/worker.py Outdated Show resolved Hide resolved
@vdbergh vdbergh force-pushed the cutechess branch 2 times, most recently from f63ee39 to 9d2a2d1 Compare July 20, 2022 16:28
@vdbergh
Copy link
Contributor Author

vdbergh commented Jul 20, 2022

Last push just moves a comment. No functional change.

@ppigazzini
Copy link
Collaborator

ppigazzini commented Jul 20, 2022

Tested working on WLS and MSYS2 with all the combinations of:

  • echo > testing/cutechess-cli
  • echo > _testing_1658323691.928943/cutechess-cli
  • zipball = "cutechess-cli-macos-64bit.zip"

If I'm not wrong the error messages are now only worker side.

@vdbergh
Copy link
Contributor Author

vdbergh commented Jul 20, 2022

If I'm not wrong the error messages are now only worker side.

Yes that is true.

But IMHO checking that cutechess is working is part of the start up of the worker (like selecting a compiler).

@ppigazzini
Copy link
Collaborator

ppigazzini commented Jul 20, 2022

I meant that I don't view entries in Events log about cutechess-cli problems:
https://dfts-0.pigazzini.it/actions

I agree that worker.py is the right module for the cutechess-cli check.

This the output worker side:

Obtaining version info for /home/usr00/_git/__test_folder00/fishtest/worker/testing/cutechess-cli ...
Unable to run cutechess-cli. Error: [Errno 8] Exec format error: '/home/usr00/_git/__test_folder00/fishtest/worker/testing/cutechess-cli'
Downloading cutechess-cli-macos-64bit.zip ...
Obtaining version info for /home/usr00/_git/__test_folder00/fishtest/worker/testing/cutechess-cli ...
Unable to run cutechess-cli. Error: [Errno 8] Exec format error: '/home/usr00/_git/__test_folder00/fishtest/worker/testing/cutechess-cli'
The downloaded cutechess-cli is not working. Trying to restore a backup copy ...
Obtaining version info for /home/usr00/_git/__test_folder00/fishtest/worker/testing/cutechess-cli ...
Unable to run cutechess-cli. Error: [Errno 8] Exec format error: '/home/usr00/_git/__test_folder00/fishtest/worker/testing/cutechess-cli'
The backup copy /home/usr00/_git/__test_folder00/fishtest/worker/_testing_1658324023.636352/cutechess-cli doesn't work either ...
No suitable cutechess-cli found
Deleting lock file /home/usr00/_git/__test_folder00/fishtest/worker/worker.lock

@vdbergh
Copy link
Contributor Author

vdbergh commented Jul 20, 2022

The messages in the event log come from failed tasks. In this PR the check for a working cutechess-cli is done before a task is started. So there are no messages in the event log.

My plan is to create a log api to post messages to the event log not associated with a task.

@ppigazzini
Copy link
Collaborator

A collateral problem found during the tests of this PR.
The worker update is checked/done in fetch_and_handle_task(), we have now some code than can prevent a fixing update.

If we add at example the support for ARM (eg the link to the cutechess-cli-arm.zip), an already installed worker will exit (because a not working cutechess) before checking the version.

@vdbergh
Copy link
Contributor Author

vdbergh commented Jul 21, 2022

I agree! So the update test should also be done a startup….

@vdbergh
Copy link
Contributor Author

vdbergh commented Jul 21, 2022

Latest push does an update test before checking cutechess.

Logic:

1) Check if cutechess-cli exists and is working.

2) If not: download it.

3) Check if downloaded cutechess-cli is working.

4) If not: try to restore a backup copy.

5) If successful check if the backup copy is working.

6) If not: bail out.

Do not treat MacOS specially in the updater.

Move cutechess-cli setup to worker.py so that it is done
only once.
@ppigazzini ppigazzini merged commit ecb1436 into official-stockfish:master Jul 21, 2022
@ppigazzini
Copy link
Collaborator

ppigazzini commented Jul 21, 2022

Nice improvement, thank you @vdbergh :)
Triggered the workers update.

PS: please try to update black. Mine is latest v 22.6.0 and writes 2**30, yours still writes 2 ** 30, see
psf/black#2726

PS2: I installed several python tools with pipx, for me is a simple pipx upgrade-all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement worker update code changes requiring a worker update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants