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

Compatibility with previous saved rsconnect.environment modules #512

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

sagerb
Copy link
Contributor

@sagerb sagerb commented Oct 19, 2023

Intent

reconnect-python saves an rsconnect.environment module within each python installation it executes from. With the removal of condo support, we need to be compatible with entries saved within those older installations.

Resolves #510

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change

Approach

the environment.py::MakeEnvironment method is passed named arguments from attributes saved within
the rsconnect.environment module. Rather than add a conda parameter, I've chosen to catch all
unmatched arguments with a wild-card catch-all.

Automated Tests

None impacted

Directions for Reviewers

This should be tested against an older python installation which had an older version of reconnect-python used with it. It should also be tested against a new installation in which the user has never used rsconnect-python.

Checklist

  • I have updated CHANGELOG.md to cover notable changes.
  • I have updated all related GitHub issues to reflect their current state.

@sagerb sagerb requested a review from mmarchetti October 19, 2023 18:01
@sagerb sagerb self-assigned this Oct 19, 2023
@sagerb sagerb marked this pull request as ready for review October 19, 2023 18:03
@github-actions
Copy link

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
4231 2775 66% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
rsconnect/bundle.py 80% 🟢
rsconnect/environment.py 81% 🟢
TOTAL 81% 🟢

updated for commit: 8e16638 by action🐍

@kgartland-rstudio
Copy link
Collaborator

Verified fixed!

I tried reproducing with the same--python path reported here and it works as expected with this new build.

I also created a fresh python environment and installed this build. I referenced the new python executable in the --python path and it too succeeds.

Copy link
Collaborator

@kgartland-rstudio kgartland-rstudio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented, but forgot to approve.

@sagerb sagerb merged commit 439c1a4 into master Oct 19, 2023
14 checks passed
@sagerb sagerb deleted the sagerb-compatible-with-older-rsconnect-environments branch October 19, 2023 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When specifying a --python path we throw a conda error
3 participants