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

delaying load of recipe until it is required by builders #358

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

grisumbras
Copy link
Contributor

Changelog: Bugfix: Fixes #331

  • Refer to the issue that supports this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.

@CLAassistant
Copy link

CLAassistant commented Mar 21, 2019

CLA assistant check
All committers have signed the CLA.

@grisumbras
Copy link
Contributor Author

I haven't touched any tests yet, just wanted to check if this is a direction that could be pursued. The basic idea is to replace code that potentially loads the recipe with code that creates objects that lazily load the recipe. As a result, the recipe isn't loaded until its attributes are required by builders, which happens after all remotes have been already added (at least in theory).

@uilianries
Copy link
Member

The idea sounds good, delaying the recipe seems be the best approach to solve python requires in CPT. Thanks for your contribution! WDYT @lasote ?

@lasote
Copy link
Contributor

lasote commented Mar 25, 2019

Sounds complex and error prone to be honest.

@grisumbras
Copy link
Contributor Author

Alternatively, we could just add necessary remotes in constructor.

@lasote
Copy link
Contributor

lasote commented Mar 25, 2019

Would be worth it to try it, yes.

@grisumbras
Copy link
Contributor Author

That is what I originally wanted to do, but then I thought, that there is a reason why adding remotes wasn't put into constructor but into run method in the first place.

@grisumbras grisumbras mentioned this pull request Mar 26, 2019
3 tasks
@lasote lasote removed their assignment Oct 29, 2019
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.

python_requires fails when remote is in CONAN_REMOTES but not added to conan
4 participants