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

Increase notebook version to >=5.7.8 in nuclei example #454

Merged

Conversation

Projects
None yet
3 participants
@BenjaminBossan
Copy link
Collaborator

commented Apr 10, 2019

This is because of a github security note. 5.7.8 seems to be the most recent version of notebook. I haven't actually checked what changed but I assume that the notebook should still work as expected.

BenjaminBossan
Increase notebook version to >=5.7.8 in nuclei example
This is because of a github security note. I haven't actually checked
what changed in the newer version.

@BenjaminBossan BenjaminBossan requested a review from ottonemo Apr 10, 2019

@BenjaminBossan BenjaminBossan self-assigned this Apr 10, 2019

@thomasjpfan
Copy link
Member

left a comment

Yup, those requirements are pretty strict, having >= in general would be a bit nicer. (It pins the versions of torch==0.4.0 and skorch==0.3.0)

@BenjaminBossan

This comment has been minimized.

Copy link
Collaborator Author

commented May 1, 2019

I wonder if it wouldn't be better to only include the dependencies needed in addition to those that skorch already requires. That way, there would be less redundancy. Of course the drawback is that this requirement.txt doesn't stand on its own anymore.

@thomasjpfan

This comment has been minimized.

Copy link
Member

commented May 1, 2019

only include the dependencies needed in addition to those that skorch already requires.

I am okay with that. The notebook will depend on skorch, so it assume the skorch dependencies are installed.

BenjaminBossan
Further update the requirements.txt for nuclei example
Only include extra requirements on top of skorch. Don't pin the exact
requirements, instead use >=.
@BenjaminBossan

This comment has been minimized.

Copy link
Collaborator Author

commented May 1, 2019

I made the discussed changes to the requirements.txt.

@thomasjpfan
Copy link
Member

left a comment

LGTM

@githubnemo

This comment has been minimized.

Copy link

commented May 3, 2019

LGTM

@BenjaminBossan BenjaminBossan merged commit 82efd5a into master May 4, 2019

@BenjaminBossan BenjaminBossan deleted the security/increase-notebook-version-nuclei-example branch May 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.