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

Set appropriate settings from a plugin context #1055

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

soapy1
Copy link
Contributor

@soapy1 soapy1 commented Jan 22, 2025

Description

Follow up to #1044

In order for a plugin to get settings at the right level of specificity, they must have information about the running environment and namespace. This PR:

  • adds environment and namespace info into the plugin context
  • updates the conda-lock plugin to use this info to get the correct conda command

Pull request checklist

  • Did you test this change locally?
  • Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

Copy link

netlify bot commented Jan 22, 2025

Deploy Preview for conda-store canceled.

Name Link
🔨 Latest commit a35228e
🔍 Latest deploy log https://app.netlify.com/sites/conda-store/deploys/6791647b085c39000732d4aa

@soapy1 soapy1 marked this pull request as draft January 22, 2025 21:35
Copy link

netlify bot commented Jan 22, 2025

Deploy Preview for conda-store canceled.

Name Link
🔨 Latest commit 4781eb8
🔍 Latest deploy log https://app.netlify.com/sites/conda-store/deploys/679920ff27a6900008a0fb50

@soapy1 soapy1 force-pushed the plugin-get-settings branch 2 times, most recently from 98d7e88 to a693995 Compare January 23, 2025 01:35
@soapy1 soapy1 marked this pull request as ready for review January 23, 2025 01:35
@soapy1 soapy1 requested a review from peytondmurray January 23, 2025 01:35
Comment on lines 84 to 87
with open(lockfile_path, "w") as f:
yaml.dump({"foo": "bar"}, f)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we even need to write to a file here?

Copy link
Contributor Author

@soapy1 soapy1 Jan 27, 2025

Choose a reason for hiding this comment

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

Good call out! Yes, we need to write a file because the last part of the lock_environment function is to read the output lockfile and return the contents to the user. So if the file isn't there an error is thrown about the missing file, for example, FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmpajob34th/conda-lock.yaml'. For the purpose of this test, it could be an empty file.

I think it makes sense for this to be an error. But open to change if that seems overly strict to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this is used in another test as well. I'll refactor some so we can reuse this in all the places it's required.

@soapy1 soapy1 force-pushed the plugin-get-settings branch from 0a790d0 to c8652f7 Compare January 27, 2025 19:57
@soapy1 soapy1 requested a review from peytondmurray January 27, 2025 19:57
@soapy1 soapy1 force-pushed the plugin-get-settings branch from 3eb042b to 1f67a02 Compare January 27, 2025 22:20
@soapy1 soapy1 force-pushed the plugin-get-settings branch from 38b984d to e3c3dc0 Compare January 27, 2025 22:30
@soapy1 soapy1 force-pushed the plugin-get-settings branch from d0bdd0c to f6f5ab3 Compare January 28, 2025 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In review 👀
Development

Successfully merging this pull request may close these issues.

2 participants