-
Notifications
You must be signed in to change notification settings - Fork 8
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: Fix snakedeploy hanging forever #67
Conversation
WalkthroughThe changes in this pull request modify the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
snakedeploy/conda.py (1)
295-295
: LGTM: Consistent addition of-y
flag.The addition of the
-y
flag to theenv remove
command in theupdate_pinning
method is consistent with the earlier change and contributes to resolving the hanging issue.For improved consistency and maintainability, consider extracting the
env remove
command with the-y
flag into a separate method within theCondaEnvProcessor
class. This would centralize the logic and make future updates easier.Here's a suggested implementation:
def remove_env(self, prefix): return self.exec_conda(f"env remove --prefix {prefix} -y") # Then use it in both methods: self.remove_env(tmpdir)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- snakedeploy/conda.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
snakedeploy/conda.py (1)
Pattern
**/*.py
: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theself
argument of methods.
Do not suggest type annotation of thecls
argument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturn
statement.
🔇 Additional comments (2)
snakedeploy/conda.py (2)
196-196
: LGTM: Addition of-y
flag addresses the hanging issue.The addition of the
-y
flag to theenv remove
command is an excellent solution to prevent the process from hanging while waiting for user input. This change directly addresses the issue described in #63 and aligns perfectly with the PR objectives.
Line range hint
1-395
: Summary: PR successfully addresses the hanging issue.The changes made in this PR effectively resolve the issue of
snakedeploy pin-conda-envs
hanging indefinitely, as described in issue #63. The addition of the-y
flag to theenv remove
commands in both theget_pkg_versions
andupdate_pinning
methods ensures that the process doesn't wait for user input, allowing it to complete successfully.These modifications align perfectly with the PR objectives and provide a straightforward solution to the problem. The changes are minimal and focused, maintaining the overall structure and logic of the
CondaEnvProcessor
class.A minor suggestion has been made to improve code maintainability by extracting the
env remove
command into a separate method. This would centralize the logic and make future updates easier, but it's not critical for the current fix.Overall, this PR successfully addresses the reported issue and improves the functionality of the
snakedeploy
tool.
🤖 I have created a release *beep* *boop* --- ## [0.10.2](v0.10.1...v0.10.2) (2024-10-16) ### Bug Fixes * Fix snakedeploy hanging forever ([#67](#67)) ([f6d1ce5](f6d1ce5)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This PR fixes #63 by adding
-y
to theenv remove
calls. It seems like the conda frontend is waiting for user input and therefore taking forever.Summary by CodeRabbit
New Features
Bug Fixes