Skip to content

Conversation

@mattem
Copy link
Collaborator

@mattem mattem commented Apr 19, 2022

Fixes #5

The virtual env is now created in an action and passed back into the runfiles. There are a few hoops to jump through because of this, mainly around dealing with pathing changes and bazel validating files (symlinks) within tree artifacts.

This should speed up running binaries and tests as changes to first party sources won't cause the venv to get recreated on each run.

For the IDE, bazel run //py/tests/external-deps:main.venv will create a virtual env for the target under {BUILD_WORKSPACE_DIRECTORY}/.{name}.venv. As a follow up, it would be nice to allow changing the path to the venv, but this is likely fine for now.

@mattem mattem force-pushed the feat/venv_action branch 4 times, most recently from 1f5512c to 6e286df Compare April 26, 2022 00:41
@mattem mattem marked this pull request as ready for review April 26, 2022 00:50
@mattem mattem requested a review from alexeagle April 26, 2022 00:50
@alexeagle alexeagle requested a review from kormide April 26, 2022 15:31
@mattem mattem force-pushed the feat/venv_action branch from 6e286df to 86839a7 Compare April 27, 2022 14:55
WHL_REQUIREMENTS_FILE=$(maybe_rlocation "{{WHL_REQUIREMENTS_FILE}}")

# Convenience vars for the Python virtual env that's created.
VENV_LOCATION="{{VENV_LOCATION}}"
Copy link
Member

Choose a reason for hiding this comment

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

hmm. perhaps make the substitutions lowercase so they are easier to differentiate from the bash vars when reading this?

VENV_LOCATION="{{venv_location}}"

Copy link
Member

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

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

🌮

@mattem mattem force-pushed the feat/venv_action branch from 86839a7 to 4b47f00 Compare April 28, 2022 01:02
@mattem mattem force-pushed the feat/venv_action branch from 4b47f00 to 1f467b2 Compare April 28, 2022 01:11
@mattem mattem merged commit 9a8e2e5 into main Apr 28, 2022
@mattem mattem deleted the feat/venv_action branch April 28, 2022 01:14
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.

Create the venv for test and binary targets in a separate action instead of during the launcher each time

4 participants