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

Parametrize DIRAC group #175

Merged
merged 6 commits into from
Nov 7, 2022
Merged

Conversation

Bilokin
Copy link
Contributor

@Bilokin Bilokin commented Oct 31, 2022

Hi @meliache,

recently, the group policy in gbasf2 was diversified, so the users can now submit the jobs from different DIRAC groups, which have different queue priorities.
And since there was created a group belle_systematics we would like to add the corresponding functionality to the b2luigi, which is extremely useful for the systematics framework.
To submit a project from a group other than belle, one has to do the following:

  • Create proxy with the custom group
  • Add a custom path to the gbasf2 command via --output_path
  • Project download should be done with the same custom DIRAC group
  • The project name should be explicitly appended to the temporary download path, because it is not added by default for a custom group

In this PR I added two optional b2luigi parameters, gbasf2_proxy_group and gbasf2_project_lpn_path.
The latter one is required if the first one is supplied.

@github-actions github-actions bot added the needs changelog Entry to CHANGELOG.md is missing label Oct 31, 2022
@meliache
Copy link
Collaborator

Cool, thanks 🙏. Just looking from my phone this looks good to me. I suggest also documenting this in the class comment for the Gbasf2Process at the top of the file, somewhere there is a listing of the important settings for gbasf2 and this will appear on the online sphinx docs. And also add this to the changelog.md under the |Unpublished (meaning not to yet in a release) section. The latter I can also do as a maintainer.

@github-actions github-actions bot removed the needs changelog Entry to CHANGELOG.md is missing label Nov 2, 2022
@Bilokin
Copy link
Contributor Author

Bilokin commented Nov 2, 2022

Thanks, I wrote the docs and the entry in the changelog, I hope it is correct.
There is an issue with the workflow though and I have no clues about it. Can you take a look?

@Bilokin
Copy link
Contributor Author

Bilokin commented Nov 7, 2022

Hi @meliache, can you take a look at the github workflow issue and the PR in general?

@meliache meliache self-assigned this Nov 7, 2022
@meliache meliache added enhancement New feature or request gbasf2 Concerns the gbasf2/grid b2luigi wrapper labels Nov 7, 2022
@meliache
Copy link
Collaborator

meliache commented Nov 7, 2022

Sorry @Bilokin, last week I just had no time to look at this because we were aiming for unblinding of our analysis for PRL, so I didn't look at this properly yet.

There is an issue with the workflow though and I have no clues about it. Can you take a look?

Yep, can take a look. Our github action runs the syntax- and style-checker flake8 as a precommit-hook and somehow that fails, but not due to your code but something with that hook. I hadn't seen that before, possibly it's not a mistake on your side, I will check.

@meliache
Copy link
Collaborator

meliache commented Nov 7, 2022

The flake8 pre-commit hook seems to fail due to an incompatibility between flake8 v3.9, which was set in the pre-commit config, and importlib_metadata version 5. I found this by googling as python/importlib_metadata#406. I created a PR which upgrades the pre-commit flake8 version to the latest version in #176. This will hopefully fix this issue. If not, I might need to further set importlib_metadata < 5.0.0 in the github action configuration

@meliache meliache merged commit 155ffc8 into nils-braun:main Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gbasf2 Concerns the gbasf2/grid b2luigi wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants