-
Notifications
You must be signed in to change notification settings - Fork 18
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(executor): override default resources to remove mem/disk (#91) #91
fix(executor): override default resources to remove mem/disk (#91) #91
Conversation
9471069
to
283045d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works nicely 👍
$ rcg logs -w root6-roofit-snakemake-kubernetes.1 | grep resources:
resources: mem_mb=1000, mem_mib=954, disk_mb=1000, disk_mib=954, tmpdir=<TBD>, kubernetes_memory_limit=256Mi
resources: mem_mb=1000, mem_mib=954, disk_mb=1000, disk_mib=954, tmpdir=<TBD>, kubernetes_memory_limit=256Mi
resources: mem_mb=1000, mem_mib=954, disk_mb=1000, disk_mib=954, tmpdir=/tmp
$ rcg logs -w root6-roofit-snakemake-kubernetes.2 | grep resources:
resources: tmpdir=<TBD>, kubernetes_memory_limit=256Mi
resources: tmpdir=<TBD>, kubernetes_memory_limit=256Mi
resources: tmpdir=/tmp
Left a minor note on the explaining comment.
Since this is a well-isolated fix, perhaps it should go to maint-0.9
as well...
config=workflow_parameters, | ||
workdir=workflow_workspace, | ||
keep_logger=True, | ||
# Since Snakemake v7.3.0, if default resources are not specified when in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A suggestion for the comment:
# Since Snakemake v7.3.0, the workflow logs include Snakemake-percieved native
# resource information on memory and storage (`mem_mb`, `disk_mb`) for each
# Snakemake rule run on cloud. However, REANA overrides these when running
# user jobs, so we should hide these in order not to present any misleading
# information to users. For this reason, the default resources are overridden
# here with the only the "bare" ones (`tmpdir`).
283045d
to
ec1ec20
Compare
ec1ec20
to
572a83f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## maint-0.9 #91 +/- ##
============================================
- Coverage 3.82% 3.78% -0.05%
============================================
Files 6 6
Lines 183 185 +2
============================================
Hits 7 7
- Misses 176 178 +2
|
* maint-0.9: fix(executor): override default resources to remove mem/disk (reanahub#91)
* fix(executor): override default resources to remove mem/disk (reanahub#91)
Closes #90