-
Notifications
You must be signed in to change notification settings - Fork 18
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 result files for deployment #263
Merged
laraPPr
merged 10 commits into
EESSI:develop
from
trz42:use_result_files_for_deployment
Mar 27, 2024
Merged
use result files for deployment #263
laraPPr
merged 10 commits into
EESSI:develop
from
trz42:use_result_files_for_deployment
Mar 27, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- defines a number of constant strings (to be used in tasks/deploy.py) - defines a new function determine_job_id_from_job_directory - defines a new function get_section_from_file - removes function read_job_metadata_from_file (calls to be replaced by get_section_from_file)
- includes numerous related changes - uses job_metadata.get_section_from_file to read job metadata and return PR section - renames determine_eessi_tarballs to determine_artefacts - uses _bot_jobSLURM_JOBID.result file to obtain artefacts - it relies on what the script bot/check-build.sh in the target GitHub repository determines as artefacts, thus it does need to know anything about the target GitHub repository - renames check_build_status to check_job_status - completely overhauled logic which essentially relies on the value of the status attribute in the _bot_jobSLURM_JOBID.result file - it relies on what the script bot/check-build.sh in the target GitHub repository determines as status, thus it does need to know anything about the target GitHub repository - renames upload_tarball to upload_artefact - no changes of logic - mostly renamed 'tarball' to 'artefact' (except for tarball_prefix* related variables; these might be changed in a later PR; a change would also require changes to the configurations, app.cfg.example and README.md) - also changed 'build_target' to 'payload' and made clear that 'eessi-VERSION-COMPONENT-OS-ARCH' is just format used within EESSI to describe the contents of a payload (or build_target) - updated uploaded_before - no changes of logic - uses 'payload' instead of 'build_target' - updated determine_successful_jobs - removes unnecessary function calls: determine_slurm_out - removes unnecessary key/value pair in job dictionary: 'slurm_out' - switches to using 'artefact' instead of 'eessi_tarball' for variables and functions (determine_artefacts) - uses renamed and overhauled check_job_status - minor polishing of log messages: 'build' -> 'job' - renames determine_tarballs_to_deploy to determine_artefacts_to_deploy - removes mentioning of 'slurm_out' data in job dictionary (docstring) - rephrases 'build*target' to 'payload' (docstring) - rephrases '(built/eessi) tarballs' to 'artefacts' (docstring, variables, log messages) - renames 'tb0' to 'artefact' - updated deploy_built_artefacts - uses determine_artefacts_to_deploy instead of determine_tarballs_to_deploy - rephrases 'target' to 'payload' - uses upload_artefact instead of upload_tarball
- file 'tools/job_metadata.py' - add constant for 'TEST' section - file 'eessi_bot_job_manager.py' - replace read_job_metadata_from_file, read_job_test and read_job_result with get_section_from_file - some polishing: changing imports, removing unused functions - file 'tests/test_tools_job_metadata.py' - replaced read_job_metadata_from_file with get_section_from_file - updated log file name accordingly
laraPPr
reviewed
Mar 25, 2024
laraPPr
approved these changes
Mar 27, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a long awaited PR for making the bot more agnostic to what it builds for. Specifically this removes code in the deployment procedure that was only correct for deploying tarballs created by build jobs for the (EESSI) software-layer.
It contains necessary changes in the logic but also some improvements to reduce boiler-plate code (handling/reading of information from
ini
/cfg
-type files) and some renaming of variables, functions and log messages.Below is a breakdown of the major changes
file
tasks/deploy.py
get_section_from_file
(tools/job_metadata.py
) to read job metadata and returnPR
section
determine_eessi_tarballs
todetermine_artefacts
_bot_jobSLURM_JOBID.result
file to obtain list of artefacts (previously named tarballs)bot/check-build.sh
in the target GitHubrepository determines as artefacts, thus it does need to know anything about
the target GitHub repository
check_build_status
tocheck_job_status
attribute
status
in the_bot_jobSLURM_JOBID.result
filebot/check-build.sh
in the target GitHubrepository determines as status, thus it does need to know anything about
the target GitHub repository
upload_tarball
toupload_artefact
tarball_prefix*
relatedvariables; these might be changed in a later PR; a change would also require
changes to the configurations of bot instances,
app.cfg.example
andREADME.md
)'eessi-VERSION-COMPONENT-OS-ARCH' is just a format used within EESSI to
describe the contents of a 'payload' (or 'build_target')
uploaded_before
determine_successful_jobs
determine_slurm_out
functions (
determine_artefacts
)check_job_status
determine_tarballs_to_deploy
todetermine_artefacts_to_deploy
messages)
deploy_built_artefacts
determine_artefacts_to_deploy
instead ofdetermine_tarballs_to_deploy
upload_artefact
instead ofupload_tarball
file
tools/job_metadata.py
tasks/deploy.py
)determine_job_id_from_job_directory
get_section_from_file
read_job_metadata_from_file
(calls to be replaced byget_section_from_file
)file
eessi_bot_job_manager.py
read_job_metadata_from_file
,read_job_test
andread_job_result
with functionget_section_from_file
file
tests/test_tools_job_metadata.py
read_job_metadata_from_file
with functionget_section_from_file
files
README.md
,app.cfg.example
,scripts/eessi-upload-to-staging
,tasks/deploy.py
andtests/test_app.cfg
tarball
withartefact
andTARBALL
withARTEFACT
, respectively (wherever it makes sensetarball
where it makes sense)files
README.md
,app.cfg.example
andtests/test_app.cfg
bot/check-build.sh
script in the target repository of a pull request and the bot does not need to know anything about those specific messages ... it just prints whatever thebot/check-build.sh
script generatesTODOs
app.cfg
; to ensure that those updates are made we should enhance the event handler and the job manager via a separate PR as described in check for needed configuration settings at the start of bot components #264