The schema validation does handle validating the value in the config (I guess it didn't when this code was written), and encrypt_data isn't used anywhere besides in setting the encrypt parameter in uploading the logs to S3, so I just cut out the variable assignment altogether and just directly plug the value from the pipeline config into the S3 call https://github.com/FCP-INDI/C-PAC/blob/d463fd3d095687bfbfae5c5cf99effb10a2d50ef/CPAC/pipeline/cpac_pipeline.py#L765
Tests
I was going to write tests for this, but I don't think we need any:
The schema validator already tests for a valid value for s3_encryption and will have a default imported if one isn't set.
If the config somehow gets switched back from c (the pipeline config) to config (the Nipype config), the AttributeError will raise, but only if the pipeline config is uploading to S3 (otherwise it doesn't matter).
Checklist
[x] My pull request has a descriptive title (not a vague title like Update index.md).
[x] My pull request targets the initial-run/ruff branch of the repository.
[x] My commit messages follow best practices.
[x] My code follows the established code style of the repository.
[ ] I added tests for the changes I made (if applicable).
[x] I updated the changelog.
[ ] I added or updated documentation (if applicable).
[ ] I tried running the project locally and verified that there are no visible errors.
[x] I ran the smoke tests and verified that they now succeed.
Developer Certificate of Origin
Developer Certificate of Origin
```
Developer Certificate of Origin
Version 1.1
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129
Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
```
Fixes
Fixes preliminary smoke test failures by @shnizzedy
Description
In https://github.com/FCP-INDI/C-PAC/blob/1fe21e19af34c058ac7455f546be948f7e5ed225/CPAC/pipeline/cpac_pipeline.py#L361-L366
config
is the Nipype configuration, not the pipeline configuration, so the broad exception was catching anAttributeError
, failing silently and always settingencrypt_data
toFalse
.Upon the linted change listing expected exceptions https://github.com/FCP-INDI/C-PAC/blob/7aaf4393efc7bf24da3c455ca171fdf329d503cb/CPAC/pipeline/cpac_pipeline.py#L364-L368
AttributeError
isn't caught, so the rule-excepted broad exception in https://github.com/FCP-INDI/C-PAC/blob/7aaf4393efc7bf24da3c455ca171fdf329d503cb/CPAC/pipeline/cpac_runner.py#L652-L665 catches the error and exits with an exit code 1.The schema validation does handle validating the value in the config (I guess it didn't when this code was written), and
encrypt_data
isn't used anywhere besides in setting theencrypt
parameter in uploading the logs to S3, so I just cut out the variable assignment altogether and just directly plug the value from the pipeline config into the S3 call https://github.com/FCP-INDI/C-PAC/blob/d463fd3d095687bfbfae5c5cf99effb10a2d50ef/CPAC/pipeline/cpac_pipeline.py#L765Tests
I was going to write tests for this, but I don't think we need any:
s3_encryption
and will have a default imported if one isn't set.c
(the pipeline config) toconfig
(the Nipype config), theAttributeError
will raise, but only if the pipeline config is uploading to S3 (otherwise it doesn't matter).Checklist
Update index.md
).initial-run/ruff
branch of the repository.Developer Certificate of Origin
Developer Certificate of Origin
``` Developer Certificate of Origin Version 1.1 Copyright (C) 2004, 2006 The Linux Foundation and its contributors. 1 Letterman Drive Suite D4700 San Francisco, CA, 94129 Everyone is permitted to copy and distribute verbatim copies of this license document, but changing it is not allowed. Developer's Certificate of Origin 1.1 By making a contribution to this project, I certify that: (a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or (b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or (c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it. (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. ```