Skip to content

change logic that checks if filter matches context in which an action shall be performed #310

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

Open
wants to merge 30 commits into
base: develop
Choose a base branch
from

Conversation

trz42
Copy link
Contributor

@trz42 trz42 commented Mar 27, 2025

This PR implements the following changes

  • changes logic for checking if a given filter (or set of filters) matches a context
  • adds more tests for tools/filters.py
  • determines coverage of all tests

Ready for review after completed all tasks below

  • functionality should be tested $\rightarrow$ tested via test bot PR310 trz42/software-layer#90
    • implemented a few fixes and improvements while testing
  • unit tests may need to be adjusted to the new logic
    • some unit tests may be obsolete due to the changed logic, e.g., accelerator filters are not checked any longer (only architecture, instance and repository are checked)
  • potential improvement: rename check_filters to something like check_build_filters because the method is very specific to be used for bot: build commands
  • potentially add a method that more flexibly checks filters by providing a list of required components
  • drop obsolete tests, or adjust them in case more flexible filter method was implemented
  • add more tests that check failures

This PR primarily changes the logic that checks if an action filter, for example, defined by arch:x86_64/generic inst:eessi-bot-mc-aws repo:eessi.io-2023.06-software in a command such as

bot: build arch:x86_64/generic inst:eessi-bot-mc-aws repo:eessi.io-2023.06-software

matches the context in which the action (build in this example) shall be performed.

The changes shall avoid accidentally triggered actions. These changes have become more and more important with the growing number of bot instances and the growing number of potential build targets. Without these changes a simple command such as

bot: build

where no filter is provided would lead to submitting build jobs for all configured build targets on all running bot instances (unless a bot instance would employ other restrictions such as only accepting commands from specific GitHub users).

The PR implements the following changes:

  • all three components architecture, instance and repository have to be provided for a filter,
  • the context against which a filter is checked also must have all three components defined (this should always be the case due to the implementation, but it is also checked)
  • the values of the three components in a filter have to be identical to the values of the corresponding component in the context (before a substring provided by a filter could match the value of the component in the context, e.g., architecture:zen would match zen2, zen3 and zen4 values for the architecture component in a context)

Since the checks are now based on exact string matches, the PR renamed the use of 'pattern' to 'value'.

truib added 25 commits March 27, 2025 23:19
This reverts commit c78feb0.

We take a different approach aiming at only modifying the existing tests such
that they continue to test what they originally were built to test, and add new
tests that cover new or modified capabilities.
- also improve docstring
- add a potential TODO
@trz42 trz42 marked this pull request as ready for review April 21, 2025 17:48
@trz42 trz42 mentioned this pull request Jul 1, 2025
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.

2 participants