LbAPCommon issueshttps://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues2023-04-14T10:28:12+02:00https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/22n_test_lfns gets overridden when using defaults2023-04-14T10:28:12+02:00Chris Burrn_test_lfns gets overridden when using defaultsSee https://gitlab.cern.ch/lhcb-datapkg/AnalysisProductions/-/merge_requests/483/diffs#note_6624357See https://gitlab.cern.ch/lhcb-datapkg/AnalysisProductions/-/merge_requests/483/diffs#note_6624357https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/7Improve test coverage of LbAPCommon.checks2023-03-07T17:10:12+01:00Chris BurrImprove test coverage of LbAPCommon.checksCurrently much of `LbAPCommon.checks` isn't covered by unit tests but this should be fixed to make it easier to maintain long-term.
See: https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/merge_requests/27#note_4641433Currently much of `LbAPCommon.checks` isn't covered by unit tests but this should be fixed to make it easier to maintain long-term.
See: https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/merge_requests/27#note_4641433Giulia TuciDylan Jaide WhiteGiulia Tucihttps://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/15Jinja exceptions are not exposed in the user-facing CI log.2023-03-07T11:09:30+01:00Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chJinja exceptions are not exposed in the user-facing CI log.Pointed out by @cburr
Currently an exception raised by Jinja will appear in the CI as
```bash
INFO:Creating production B02DDbarKPi
FATAL:Error creating production: Failed to create production for B02DDbarKPi
WARN:not enough values to u...Pointed out by @cburr
Currently an exception raised by Jinja will appear in the CI as
```bash
INFO:Creating production B02DDbarKPi
FATAL:Error creating production: Failed to create production for B02DDbarKPi
WARN:not enough values to unpack (expected 7, got 5)
```
If one were to test locally they would see
```bash
Traceback (most recent call last):
File "/miniconda/envs/analysis-productions/lib/python3.9/site-packages/LbAnalysisProductions/gitlab_runner.py", line 200, in create_productions
production = Production.create(
File "/miniconda/envs/analysis-productions/lib/python3.9/site-packages/LbAnalysisProductions/models/production.py", line 64, in create
self.rendered_yaml = LbAPCommon.render_yaml(self.yaml)
File "/miniconda/envs/analysis-productions/lib/python3.9/site-packages/LbAPCommon/parsing.py", line 187, in render_yaml
rendered_yaml = jinja2.Template(
File "/miniconda/envs/analysis-productions/lib/python3.9/site-packages/jinja2/environment.py", line 1291, in render
self.environment.handle_exception()
File "/miniconda/envs/analysis-productions/lib/python3.9/site-packages/jinja2/environment.py", line 925, in handle_exception
raise rewrite_traceback_stack(source=source)
File "<template>", line 132, in top-level template code
ValueError: not enough values to unpack (expected 7, got 5)
```
which specifies the line causing the exception so is much more useful for a user to debug their YAML.
We should try have a message like `Exception running jinja2 templating on line 132 of xxx/yyy.yaml: ValueError: not enough values to unpack (expected 7, got 5)`Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chAidan Richard Wiederholdaidan.richard.wiederhold@cern.chhttps://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/12Ensure that files our tests depend on are preserved2023-01-30T16:02:52+01:00Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chEnsure that files our tests depend on are preservedI doubt this is much of an issue but for the sake of safety it might be good if we had somewhere to store root files used in tests (such as https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/blob/master/tests/checks/test_s...I doubt this is much of an issue but for the sake of safety it might be good if we had somewhere to store root files used in tests (such as https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/blob/master/tests/checks/test_simple.py#L24) to ensure that we have control over them and prevent the tests from failing one day because someone else deleted a file we happen to test on.
Maybe they can just be stored in the repository like https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/blob/master/tests/checks/example_tuple_with_lumi.root but that's probably not ideal depending on how large they are.https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/21Finalise test coverage of LbAPCommon.checks2022-08-16T15:27:30+02:00Eduardo RodriguesFinalise test coverage of LbAPCommon.checksFollows from https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/7 towards 100% coverage.Follows from https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/7 towards 100% coverage.2023-01-31https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/13Add post-job validation unit tests2022-08-16T12:19:30+02:00Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chAdd post-job validation unit testsWe currently have unit tests to cover the yaml validation etc... that just study the input of a job but some validation methods (similar to checks) now rely on studying the output of a job rather than just its input. It should be enough ...We currently have unit tests to cover the yaml validation etc... that just study the input of a job but some validation methods (similar to checks) now rely on studying the output of a job rather than just its input. It should be enough to set up a job specifically for these tests and to store their relevant output in the repository (if they're not too large) for the validation to then study.
Work towards https://gitlab.cern.ch/groups/lhcb-dpa/-/epics/13.
@erodrigu this should help improve our overall coverage.Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chAidan Richard Wiederholdaidan.richard.wiederhold@cern.chhttps://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/19Some check types should have failed checks combined and reassessed based on t...2022-08-16T12:10:30+02:00Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chSome check types should have failed checks combined and reassessed based on the combination - lhcb-dpa/project#93As discussed [here](https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/merge_requests/61#note_5588655) where some checks fail on a single file, when combining the results from multiple files the overall failure/success sho...As discussed [here](https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/merge_requests/61#note_5588655) where some checks fail on a single file, when combining the results from multiple files the overall failure/success should be reassessed based on the combined results.Giulia TuciDylan Jaide WhiteGiulia Tucihttps://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/18Revive options validation and add it to pipelines - for lhcb-dpa/project#932022-06-15T13:32:11+02:00Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chRevive options validation and add it to pipelines - for lhcb-dpa/project#93[Options validation](https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/blob/master/src/LbAPCommon/options_parsing/__init__.py#L19) was previously created to check that the trees users expected were present in their output...[Options validation](https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/blob/master/src/LbAPCommon/options_parsing/__init__.py#L19) was previously created to check that the trees users expected were present in their output files however this has become [incompatible with lb-prod-run](https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbaplocal/-/blob/master/src/LbAPLocal/cli.py#L270) so we should update it.
It would also be worth adding it to the pipeline tests.Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chAidan Richard Wiederholdaidan.richard.wiederhold@cern.chhttps://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/11Reduce the possibility for accidental bk_query duplication by adding a new ya...2022-06-15T13:32:03+02:00Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chReduce the possibility for accidental bk_query duplication by adding a new yaml fieldIn general a user will not need to use a given `bk_query` twice in a single production version, there are however possible legitimate use cases. Currently a user will just receive a warning if they duplicate a `bk_query` but it would be ...In general a user will not need to use a given `bk_query` twice in a single production version, there are however possible legitimate use cases. Currently a user will just receive a warning if they duplicate a `bk_query` but it would be better to fail yaml validation when a duplicate `bk_query` is detected unless the user makes it clear they intend to do this by use of a new yaml field boolean `allow_duplicate_bk_query`.
As discussed in lhcb-dpa/analysis-productions/lbapcommon!35 concerning the lhcb-datapkg/AnalysisProductions!151 production which does need to duplicate `bk_query` entries but was being blocked by the restriction on duplicates.Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chAidan Richard Wiederholdaidan.richard.wiederhold@cern.chhttps://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/issues/16Checks need to handle array branches differently2022-03-10T15:37:23+01:00Aidan Richard Wiederholdaidan.richard.wiederhold@cern.chChecks need to handle array branches differentlyCurrently if one tries to run a range check on a branch of arrays you can get the error
```python
Traceback (most recent call last):
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/bin/lb-ap", line 8, in <module>
s...Currently if one tries to run a range check on a branch of arrays you can get the error
```python
Traceback (most recent call last):
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/bin/lb-ap", line 8, in <module>
sys.exit(main())
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/lib/python3.9/site-packages/click/core.py", line 1128, in __call__
return self.main(*args, **kwargs)
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/lib/python3.9/site-packages/click/core.py", line 1053, in main
rv = self.invoke(ctx)
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/lib/python3.9/site-packages/click/core.py", line 1659, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/lib/python3.9/site-packages/click/core.py", line 1395, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/lib/python3.9/site-packages/click/core.py", line 754, in invoke
return __callback(*args, **kwargs)
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/lib/python3.9/site-packages/LbAPLocal/cli.py", line 357, in ci_checks
check_results = run_job_checks(
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/lib/python3.9/site-packages/LbAPCommon/checks.py", line 46, in run_job_checks
check_results[check] = range_check(
File "/cvmfs/lhcb.cern.ch/lib/var/lib/LbEnv/2361/stable/linux-64/lib/python3.9/site-packages/LbAPCommon/checks.py", line 232, in range_check
(test_array < limits["max"]) & (test_array > limits["min"])
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
```
so we need a way to check which events satisfy the given range and record the values when the branch to be checked is an array.