-
Notifications
You must be signed in to change notification settings - Fork 17.7k
Optimizes building of Production image for non-modified www files #19210
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| # | ||
| """Utilities for creating a virtual environment""" | ||
| import os | ||
| import shutil | ||
| import sys | ||
| from collections import deque | ||
| from typing import List, Optional | ||
|
|
@@ -27,13 +28,22 @@ | |
| from airflow.utils.process_utils import execute_in_subprocess | ||
|
|
||
|
|
||
| def _generate_virtualenv_cmd(tmp_dir: str, python_bin: str, system_site_packages: bool) -> List[str]: | ||
| cmd = [sys.executable, '-m', 'virtualenv', tmp_dir] | ||
| if system_site_packages: | ||
| cmd.append('--system-site-packages') | ||
| if python_bin is not None: | ||
| cmd.append(f'--python={python_bin}') | ||
| return cmd | ||
| def _generate_virtualenv_cmd( | ||
| tmp_dir: str, python_bin: str, system_site_packages: bool, clone_virtualenv_packages: bool | ||
| ) -> List[str]: | ||
| if clone_virtualenv_packages and sys.prefix != sys.base_prefix and not python_bin: | ||
| # Create virtualenv using virtualenv-clone command if we are in virtualenv and | ||
| # clone_virtualenv_packages is set and we are using same version of python | ||
| # as our virtualenv | ||
| cmd = ['virtualenv-clone', f'{sys.prefix}', tmp_dir] | ||
| return cmd | ||
| else: | ||
| cmd = [sys.executable, '-m', 'virtualenv', tmp_dir] | ||
| if system_site_packages: | ||
| cmd.append('--system-site-packages') | ||
| if python_bin is not None: | ||
| cmd.append(f'--python={python_bin}') | ||
| return cmd | ||
|
|
||
|
|
||
| def _generate_pip_install_cmd(tmp_dir: str, requirements: List[str]) -> Optional[List[str]]: | ||
|
|
@@ -75,7 +85,11 @@ def remove_task_decorator(python_source: str, task_decorator_name: str) -> str: | |
|
|
||
|
|
||
| def prepare_virtualenv( | ||
| venv_directory: str, python_bin: str, system_site_packages: bool, requirements: List[str] | ||
| venv_directory: str, | ||
| python_bin: str, | ||
| system_site_packages: bool, | ||
| requirements: List[str], | ||
| clone_virtualenv_packages: bool = False, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel we may want to infer this from |
||
| ) -> str: | ||
| """ | ||
| Creates a virtual environment and installs the additional python packages | ||
|
|
@@ -89,10 +103,17 @@ def prepare_virtualenv( | |
| :type system_site_packages: bool | ||
| :param requirements: List of additional python packages | ||
| :type requirements: List[str] | ||
| :param clone_virtualenv_packages: whether to clone aurflow virtualenv package if airflow is run | ||
| in virtualenv - default is False | ||
| :type clone_virtualenv_packages: bool | ||
| :return: Path to a binary file with Python in a virtual environment. | ||
| :rtype: str | ||
| """ | ||
| virtualenv_cmd = _generate_virtualenv_cmd(venv_directory, python_bin, system_site_packages) | ||
| virtualenv_cmd = _generate_virtualenv_cmd( | ||
| venv_directory, python_bin, system_site_packages, clone_virtualenv_packages | ||
| ) | ||
| # Virtualenv-clone requires the directory to be non-existing | ||
| shutil.rmtree(path=venv_directory, ignore_errors=True) | ||
|
Comment on lines
115
to
116
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may want to explicitly error out here instead to avoid the user accidentally nuking something. A |
||
| execute_in_subprocess(virtualenv_cmd) | ||
| pip_cmd = _generate_pip_install_cmd(venv_directory, requirements) | ||
| if pip_cmd: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should
python_versionif it matchessys.version_info...?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AH yeah - that's the "previous" PR (this one is based on it) - I will come back to it shortly and address the comments there :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the #19189 - but I will take your comments and answer here and address them there.