Skip to content

reorganisation of constants and validation of config settings#266

Merged
Neves-P merged 5 commits intoEESSI:developfrom
trz42:reorganise_constants_and_validate_config
May 6, 2024
Merged

reorganisation of constants and validation of config settings#266
Neves-P merged 5 commits intoEESSI:developfrom
trz42:reorganise_constants_and_validate_config

Conversation

@trz42
Copy link
Copy Markdown
Contributor

@trz42 trz42 commented Apr 22, 2024

This PR turned out to be larger than originally intended (just validating that required config settings are defined when bot components start). However, it reorganises / brings consistent structure to how and where string constants are defined. Hopefully, this makes the code easier to read.

  • string constants being used have been defined in the correct modules
    • when they are used they have to be prefixed with the module, hence it becomes more clear to which module they belong
  • a new file tools/cvmfs_repository.py for constants used for the settings of a CernVM-FS repository is added
  • both the event handler and job manager verify at the start that required/recommended settings are defined in 'app.cfg'

TODO

- string constants being used have been defined in the correct modules
  - when they are used they have to be prefixed with the module, hence
    it becomes more clear to which module they belong
- a new file tools/cvmfs_repository.py for constants used for the settings of a
  CernVM-FS repository is added
- both the event handler and job manager verify at the start that
  required/recommended settings are defined in 'app.cfg'
Copy link
Copy Markdown
Member

@Neves-P Neves-P left a comment

Choose a reason for hiding this comment

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

Note: I have not run the bot instance on our site with this PR yet, I have only reviewed the code.

I think it's a good change and it adds needed structure for the bot configuration which will make it easier to setup on other sites too! I've just noted a few nitpicking things which could just be me not understanding the logic, go ahead and reject if that's the case.

Comment thread tools/cvmfs_repository.py Outdated
Comment thread eessi_bot_event_handler.py
Comment thread tools/job_metadata.py
Comment thread tools/job_metadata.py Outdated
Copy link
Copy Markdown
Member

@Neves-P Neves-P left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@Neves-P Neves-P merged commit 02fd520 into EESSI:develop May 6, 2024
@trz42 trz42 mentioned this pull request May 15, 2024
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.

3 participants