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

Addressing NumPy security alert #530

Merged
merged 4 commits into from
Feb 8, 2022

Conversation

john-science
Copy link
Member

@john-science john-science commented Jan 11, 2022

NumPy has released the following security alert: GHSA-6p56-wp2h-9hxr

This PR addresses that alert. Though it may affect some floating point calculations down stream, so some testing of dependent projects in the ARMI ecosystem should be done before this is merged into master.

NOTE: There is a lot of testing that will need to be done before merging. Please do not hit "Merge" this PR for me.

@john-science john-science added the enhancement New feature or request label Jan 11, 2022
requirements-testing.txt Outdated Show resolved Hide resolved
@john-science
Copy link
Member Author

john-science commented Jan 14, 2022

All the unit tests in this branch run fine on my Linux workstation. I see the old NumPy bound existed because there was a NumPy-related "bug in Windows".

I don't have the foggiest guess what that bug might be, but I will test this PR on a Windows machine before merging into the repo.

NOTE: There is a lot of testing that will need to be done before merging. Please do not hit "Merge" this PR for me.

@jakehader
Copy link
Member

@john-science are waiting on something to merge this in? I assume this will break some down stream stuff on our internal end with using previous numpy versions.

@john-science
Copy link
Member Author

@jakehader You said we needed to do some downstream testing before we could merge this. We've been too busy to talk about getting that done.

@jakehader
Copy link
Member

This will probably break some stuff downstream for us but let's just pull the trigger and then fix things as needed. Especially if this is technically a security concern we should address. Oh! One more thing - should we update the version of the framework because of this change?

@john-science
Copy link
Member Author

@jakehader If you think we can just roll with it, that's good enough for me.

And the version bump! You are totally right. I will fill out the release docs page and bump the version right now, and Merge this morning.

Cheers!

@john-science john-science merged commit f4610ba into terrapower:master Feb 8, 2022
@john-science john-science deleted the numpy_upgrade branch February 8, 2022 17:02
scottyak pushed a commit to scottyak/armi that referenced this pull request Oct 27, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants