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

Move check result to target repo (bot/check-result.sh script) #174

Merged
merged 45 commits into from
Jun 20, 2023

Conversation

trz42
Copy link
Contributor

@trz42 trz42 commented Apr 10, 2023

PR on top of #172 to improve analysing the result of a bot job. It intends to move any repository specific code for checking the result of a build job to the target repository (what the bot is used to build for). The two major changes are

  • letting the main job script run the script bot/check-result.sh provided by the target repository (e.g., EESSI/software-layer or EESSI/compatibility-layer or any other "software" repository)
  • letting the job manager's process_finished_job function use a result file produced by the above check-result.sh script to update a comment in the PR with a SUCCESS/FAILURE/UNKNOWN message and detailed information

At the moment, the code still includes the old method for analysing a job's result. Once the software-layer and compatibility-layer repositories have been updated to provide bot/check-result.sh script this can be removed. A draft version of such a script is included in EESSI/compatibility-layer#179

Rather removed all that code in favour of having a clean cut/change. Matching PRs for compatibility layer (EESSI/compatibility-layer#179) and software layer (EESSI/software-layer#241) need to be updated.

Update June 18:

TODOs

truib added 25 commits April 7, 2023 12:48
app.cfg.example
- polished/clarified notes about comment templates
- more control over format for details

eessi_bot_job_manager.py
- support for more control over format for details in PR comment
- handling case that no result file is found

tools/pr_comments.py
- support for more control over format for details in PR comment
@trz42 trz42 marked this pull request as draft April 25, 2023 08:08
- we assume that the check-result.sh script provides a fully defined HTML string
  the bot can just dump as is into the column in the status table (no further
  processing is needed)

- changes separated for each file

  - app.cfg.example:
    - removed *_fmt settings for summary, details and artefacts
    - added new format for when result is UNKNOWN (ie no result
      file found or no pre-formatted comment_details found)

  - eessi_bot_job_manager.py:
    - replaced existing constants with new constants
    - updated comment explaining expected format of result file
    - updated processing of result file (got much simpler --> complexity/work
      moved to target respository)
    - only needs to retain some code then result is not defined/unknown (format
      of this is defined via `app.cfg`)

  - scripts/bot-build.slurm:
    - adjusted format of result file that is generated if the target repository
      does not provide the bot/check-result.sh

  - tools/pr_comments.py:
    - removed function make_html_list_items which is not used/needed any longer
trz42 pushed a commit to trz42/eessi-bot-software-layer that referenced this pull request Jun 17, 2023
…com/trz42/eessi-bot-software-layer into update_eessi_with_PR174_182_185

- tools/job_metadata.py
  - added create_metadata_file (introduced by EESSI#174)
  - added read_metadata_file (introduced by EESSI#174)
  - let read_job_metadata_from_file use read_metadata_file

- eessi_bot_job_manager.py
  - made use of read_job_metadata_from_file in read_job_pr_metadata
@trz42 trz42 marked this pull request as ready for review June 18, 2023 18:12
scripts/bot-build.slurm Outdated Show resolved Hide resolved
boegel and others added 2 commits June 20, 2023 11:21
@boegel boegel changed the title Move check result to target repo Move check result to target repo (bot/check-result.sh script) Jun 20, 2023
Copy link
Contributor

@boegel boegel left a comment

Choose a reason for hiding this comment

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

Tested along with EESSI/software-layer#241 via boegel/software-layer#25, works like a charm.

Thanks (again) for all the effort on this @trz42!

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