Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @asmacdo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request aims to improve the project's maintainability and security posture by eliminating an unnecessary and problematic dependency. By removing pycrypto, the project mitigates risks associated with unmaintained software and resolves persistent CI build issues, without impacting core functionality.
Highlights
- Dependency Removal: Removed the
pycryptodependency fromsetup.pyas it appears to be unused, causes CI failures, is unmaintained, and has known vulnerabilities. - Impact Assessment: Local testing confirmed no regressions after removal, specifically for SSH-type resource login and execution, suggesting
pycryptowas not a critical active dependency.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This is an excellent change. Removing the pycrypto dependency is the correct course of action, as it is unmaintained, has known vulnerabilities, and was causing CI failures. The project already depends on cryptography, which is the modern and secure alternative. Your investigation confirming that paramiko works as expected is valuable, as it's a key consumer of cryptographic libraries for SSH. The change itself is correct. I have one minor suggestion: consider correcting the typo in the pull request title from pycrpyto to pycrypto for better history tracking.
- I did not see any direct usage of this, ie grep -rn "Crypto" - The install of pycrpyto fails in CI with: longintrepr.h: No such file or directory (installing dev python with headers did not help) - project is archived, unmaintained, and has known vulnerabilities
|
please check git log for that addition -- commit message might be the one describing why it was added (e.g. optional dependency of paramiko or smth which we had to have) |
|
This dep was added in #101 (back in the niceman days) Commit message indicates that it was necessary to login to AWS EC2. I haven't worked with reproman on EC2 yet, but expanding the tutorial to cover HTCondor and AWS is something I want to get to soon anyway. IMO
|
2 participants
Locally I didnt see any extra tests failing. This could just be a legacy requirement?
To test I created a new pyenv, created a new ssh-type resource (typhon), and I was able to
loginandexecute. I am not confident that this is sufficient, but maybe we should just remove anyway, and if something breaks we fix it with some other library.