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

Allow custom modules to handle pip-installed custom modules #1554

Merged
merged 5 commits into from
Oct 2, 2020

Conversation

conradoverta
Copy link
Contributor

Before this patch, we couldn't specify modules installed via pip but that are not available in pypi. The main fix for this is to ignore venv only if the specific desired module is not inside a venv. If it is, then we must include that folder in the custom modules and force the search path to be able to find it.

To facilitate the definition of such custom modules, as they can be pretty deep and different based on how people's envs are setup, we allow the user to pass the name of the module and we resolve to the right path.

@nhatsmrt
Copy link
Contributor

Just took a look. Everything seems solid to me, but is there a test case that we can run to verify the expected behavior?

@conradoverta
Copy link
Contributor Author

Just took a look. Everything seems solid to me, but is there a test case that we can run to verify the expected behavior?

What I had in mind is to add a package that we pip-install, add a model that depends on it and then try to deploy. It might require more coordination than this repo allows. We have another repo with end to end tests for that purpose.

@conradoverta conradoverta marked this pull request as ready for review October 2, 2020 15:56
@conradoverta conradoverta merged commit 96545f9 into master Oct 2, 2020
@conradoverta conradoverta deleted the cm/custom-module-installed branch October 2, 2020 16:47
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.

2 participants