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

fix: create jobname with wildcards of all jobs in job groups #8

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

dlaehnemann
Copy link
Contributor

  • also use job.groupid instead of job.name for log_folder in case of job groups

This fixes a KeyError on JobGroup that I got, because a JobGroup doesn't have a wildcards_dict property (only its member jobs have that property). So this pull request fixes the plugin, to allow using group jobs without throwing an error.

also use job.groupid instead of job.name for log_folder in case of job groups
@dlaehnemann
Copy link
Contributor Author

@BEFH, any capacity for a quick review? Would be great to get this in and released, so we can avoid having to hack into our plugin installations...

@BEFH BEFH merged commit 536abe4 into BEFH:main Jun 3, 2024
3 checks passed
@BEFH
Copy link
Owner

BEFH commented Jun 3, 2024

I don't actually have a CI set up to automatically release on pull request merges, but I will make a release for this shortly.

@dlaehnemann
Copy link
Contributor Author

As with the main plugin code, I think simply copying from the slurm executor should be easy enough for getting release-please to run... ;)

@dlaehnemann
Copy link
Contributor Author

@dlaehnemann dlaehnemann deleted the patch-1 branch June 3, 2024 13:53
@BEFH
Copy link
Owner

BEFH commented Jun 3, 2024

I think there's actually more new code than old in here, but I get you.

I haven't used Release Please in the past, but I may add it soon. I want to make sure there aren't requirements for commit/PR naming, that we can customize the release notes, and that it handles direct commits as well as PRs. Also, the action used in the Slurm executor is deprecated with https://github.com/googleapis/release-please-action as the new one, so there will be some slight differences.

@dlaehnemann
Copy link
Contributor Author

Ah, yes. I didn't check for the deprecation, but have seen this in another project. So it's probably worth directly using the newer one.

And release-please does assume conventional commits, which may seem like an additional burden. But I do find them useful anyways, and nowadays often use them, even when not necessary (see this PR for example).

And also, of course there's a lot of specific / unique code over here. That's the whole fun of different workflow systems (and plugins), right... 🤯

Ash1One pushed a commit to Ash1One/snakemake-executor-plugin-lsf that referenced this pull request Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants