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

tox: skip install stages are not skipping install #4591

Closed
jmahlik opened this issue Apr 17, 2024 · 0 comments · Fixed by #4603
Closed

tox: skip install stages are not skipping install #4591

jmahlik opened this issue Apr 17, 2024 · 0 comments · Fixed by #4603

Comments

@jmahlik
Copy link
Contributor

jmahlik commented Apr 17, 2024

Describe the bug
Some tox stages can skip the install of the entire testing stack. The current config is resulting in these stages installing the .[test] extra, so they are taking a lot longer than they should. Fixing should improve ci time significantly for the PR checks.

Flake8 for instance takes about 1/7 if the time.

Current config -- results in .[test] being installed.

[testenv:flake8]
skipdist = true
skip_install = true
commands =
    pip install --exists-action=w -r requirements/tox/flake8_requirements.txt
    flake8

After -- only installs the flake8 deps

[testenv:flake8]
skipdist = true
skip_install = true
deps =
    -r requirements/tox/flake8_requirements.txt
commands =
    flake8

Timings for an environment recreation. These timings are on windows, so they're a quite inflated. The relative time is important though. This is with everything in the local pip cache.

Notice the large amount of deps installed before the change

Before Change

time tox -e flake8 -r
flake8 recreate: \.tox\flake8
flake8 installdeps: .[test]
flake8 installed: accelerate==0.27.0,aiohttp==3.9.5,aiosignal==1.3.1,alembic==1.13.1,anyio==4.3.0,apache-airflow==2.8.4,apache-airflow-providers-amazon==7.2.1,apache-airflow-providers-common-io==1.3.1,apache-airflow-providers-common-sql==1.12.0,apache-airflow-providers-ftp==3.8.0,apache-airflow-providers-http==4.10.1,apache-airflow-providers-imap==3.5.0,apache-airflow-providers-smtp==1.6.1,apache-airflow-providers-sqlite==3.7.1,apispec==6.6.0,argcomplete==3.3.0,asgiref==3.8.1,asn1crypto==1.5.1,asttokens==2.4.1,atomicwrites==1.4.1,attrs==23.2.0,awslogs==0.14.0,Babel==2.14.0,bcrypt==4.1.2,beautifulsoup4==4.12.3,black==24.3.0,blinker==1.7.0,boto3==1.34.85,botocore==1.34.85,Brotli==1.1.0,cachelib==0.9.0,certifi==2024.2.2,cffi==1.16.0,charset-normalizer==3.3.2,click==8.1.7,clickclick==20.10.2,cloudpickle==2.2.1,colorama==0.4.6,colorlog==4.8.0,ConfigUpdater==3.2,connexion==2.14.2,contextlib2==21.6.0,coverage==6.1.2,cron-descriptor==1.4.3,croniter==2.0.3,cryptography==42.0.5,decorator==5.1.1,Deprecated==1.2.14,dill==0.3.8,distlib==0.3.8,dnspython==2.6.1,docker==6.1.3,docutils==0.21.1,email-validator==1.3.1,execnet==2.1.1,executing==2.0.1,fabric==2.6.0,fastjsonschema==2.19.1,filelock==3.13.4,flake8==4.0.1,Flask==2.2.5,Flask-AppBuilder==4.3.11,Flask-Babel==2.0.0,Flask-Caching==2.1.0,Flask-JWT-Extended==4.6.0,Flask-Limiter==3.5.1,Flask-Login==0.6.3,Flask-Session==0.5.0,Flask-SQLAlchemy==2.5.1,Flask-WTF==1.2.1,frozenlist==1.4.1,fsspec==2024.3.1,gevent==24.2.1,geventhttpclient==2.0.2,google-pasta==0.2.0,google-re2==1.1,googleapis-common-protos==1.63.0,greenlet==3.0.3,grpcio==1.62.1,gunicorn==22.0.0,h11==0.14.0,httpcore==1.0.5,httpx==0.27.0,huggingface-hub==0.22.2,idna==3.7,importlib-metadata==6.11.0,importlib_resources==6.4.0,inflection==0.5.1,iniconfig==2.0.0,invoke==1.7.3,ipython==8.23.0,itsdangerous==2.2.0,jedi==0.19.1,Jinja2==3.1.3,jmespath==0.10.0,joblib==1.4.0,json5==0.9.25,jsonpath-ng==1.6.1,jsonpickle==3.0.4,jsonschema==4.21.1,jsonschema-specifications==2023.12.1,jupyter_core==5.7.2,lazy-object-proxy==1.10.0,limits==3.10.1,linkify-it-py==2.0.3,lockfile==0.12.2,lxml==5.2.1,Mako==1.3.3,markdown-it-py==3.0.0,MarkupSafe==2.1.5,marshmallow==3.21.1,marshmallow-oneofschema==3.1.1,marshmallow-sqlalchemy==0.26.1,matplotlib-inline==0.1.7,mccabe==0.6.1,mdit-py-plugins==0.4.0,mdurl==0.1.2,mock==4.0.3,more-itertools==10.2.0,mpmath==1.3.0,multidict==6.0.5,multiprocess==0.70.16,mypy-boto3-appflow==1.34.0,mypy-boto3-rds==1.34.83,mypy-boto3-redshift-data==1.34.0,mypy-extensions==1.0.0,nbformat==5.10.4,networkx==3.3,numpy==1.26.4,onnx==1.16.0,opentelemetry-api==1.24.0,opentelemetry-exporter-otlp==1.24.0,opentelemetry-exporter-otlp-proto-common==1.24.0,opentelemetry-exporter-otlp-proto-grpc==1.24.0,opentelemetry-exporter-otlp-proto-http==1.24.0,opentelemetry-proto==1.24.0,opentelemetry-sdk==1.24.0,opentelemetry-semantic-conventions==0.45b0,ordered-set==4.1.0,packaging==24.0,pandas==1.4.4,paramiko==3.4.0,parso==0.8.4,pathlib2==2.3.7.post1,pathos==0.3.2,pathspec==0.12.1,pendulum==3.0.0,pillow==10.3.0,platformdirs==4.2.0,pluggy==1.4.0,ply==3.11,pox==0.3.4,ppft==1.7.6.8,prison==0.2.1,prompt-toolkit==3.0.43,protobuf==4.25.3,psutil==5.9.8,pure-eval==0.2.2,py==1.11.0,py4j==0.10.9.5,pycodestyle==2.8.0,pycparser==2.22,pyflakes==2.4.0,Pygments==2.17.2,PyJWT==2.8.0,PyNaCl==1.5.0,pyspark==3.3.1,pytest==6.2.5,pytest-cov==3.0.0,pytest-forked==1.6.0,pytest-rerunfailures==10.2,pytest-timeout==2.1.0,pytest-xdist==2.4.0,python-daemon==3.0.1,python-dateutil==2.9.0.post0,python-nvd3==0.15.0,python-rapidjson==1.16,python-slugify==8.0.4,pytz==2024.1,pyvis==0.2.1,pywin32==306,PyYAML==6.0,redshift-connector==2.1.1,referencing==0.34.0,requests==2.31.0,requests-toolbelt==1.0.0,rfc3339-validator==0.1.4,rich==13.7.1,rich-argparse==1.4.0,rpds-py==0.18.0,s3transfer==0.10.1,safetensors==0.4.3,sagemaker @ file:///sagemaker-python-sdk,sagemaker-experiments==0.1.35,sagemaker-schema-inference-artifacts==0.0.5,sagemaker_feature_store_pyspark_3.3==1.1.2,schema==0.7.5,scikit-learn==1.3.0,scipy==1.10.1,scramp==1.4.5,setproctitle==1.3.3,six==1.16.0,smdebug-rulesconfig==1.0.1,sniffio==1.3.1,soupsieve==2.5,SQLAlchemy==1.4.52,SQLAlchemy-JSONField==1.0.2,sqlalchemy-redshift==0.8.14,SQLAlchemy-Utils==0.41.2,sqlparse==0.5.0,stack-data==0.6.3,stopit==1.1.2,sympy==1.12,tabulate==0.9.0,tblib==3.0.0,tenacity==8.2.3,termcolor==2.4.0,text-unidecode==1.3,threadpoolctl==3.4.0,time-machine==2.14.1,toml==0.10.2,tomli==2.0.1,torch==2.2.2,tox==3.24.5,tqdm==4.66.2,traitlets==5.14.2,tritonclient==2.36.0,typing_extensions==4.11.0,tzdata==2024.1,uc-micro-py==1.0.3,unicodecsv==0.14.1,universal-pathlib==0.1.4,urllib3==2.2.1,virtualenv==20.25.3,watchtower==2.0.1,wcwidth==0.2.13,websocket-client==1.7.0,Werkzeug==2.2.3,wrapt==1.16.0,WTForms==3.1.2,xgboost==1.7.6,yarl==1.9.4,zipp==3.18.1,zope.event==5.0,zope.interface==6.3
flake8 run-test-pre: PYTHONHASHSEED='42'
flake8 run-test: commands[0] | pip install --exists-action=w -r requirements/tox/flake8_requirements.txt
Requirement already satisfied: flake8==4.0.1 in \.tox\flake8\lib\site-packages (from -r requirements/tox/flake8_requirements.txt (line 1)) (4.0.1)
port-0.4.6-py2.py3-none-any.whl (10 kB)
Requirement already satisfied: mccabe<0.7.0,>=0.6.0 in .tox\flake8\lib\site-packages (from flake8==4.0.1->-r requirements/tox/flake8_requirements.txt (line 1)) (0.6.1)
Requirement already satisfied: pycodestyle<2.9.0,>=2.8.0 in .tox\flake8\lib\site-packages (from flake8==4.0.1->-r requirements/tox/flake8_requirements.txt (line 1)) (2.8.0)
Requirement already satisfied: pyflakes<2.5.0,>=2.4.0 in .tox\flake8\lib\site-packages (from flake8==4.0.1->-r requirements/tox/flake8_requirements.txt (line 1)) (2.4.0)
Installing collected packages: flake8-future-import
Successfully installed flake8-future-import-0.4.6
flake8 run-test: commands[1] | flake8

real    7m1.687s
user    0m0.000s
sys     0m0.109s

Post change

time tox -e flake8 -r
flake8 recreate: .tox\flake8
flake8 installdeps: -rrequirements/tox/flake8_requirements.txt
flake8 installed: flake8==4.0.1,flake8-future-import==0.4.6,mccabe==0.6.1,pycodestyle==2.8.0,pyflakes==2.4.0
flake8 run-test-pre: PYTHONHASHSEED='42'
flake8 run-test: commands[0] | flake8

real    0m52.988s
user    0m0.000s
sys     0m0.046s

To reproduce
Run the tox linting stages. On windows, it takes a few hours just making environments which is how I noticed this.

Expected behavior
Only install the needed deps

Screenshots or logs
n/a

System information
A description of your system. Please provide:

  • SageMaker Python SDK version: master
  • Framework name (eg. PyTorch) or algorithm (eg. KMeans): n/a
  • Framework version: n/a
  • Python version: n/a
  • CPU or GPU: n/a
  • Custom Docker image (Y/N): n/a

Additional context
I can submit a PR with changes.

jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Apr 17, 2024
… needed

Moved to using deps to override the test extra being installed in
skip_install tox stages. Now only the needed deps are installed.

Improves install time by around 7x.

Closes aws#4591
jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Apr 22, 2024
… needed

Moved to using deps to override the test extra being installed in
skip_install tox stages. Now only the needed deps are installed.

Improves install time by around 7x.

Closes aws#4591
jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Apr 22, 2024
… needed

Moved to using deps to override the test extra being installed in
skip_install tox stages. Now only the needed deps are installed.

Improves install time by around 7x.

Closes aws#4591
@jmahlik jmahlik mentioned this issue Apr 22, 2024
9 tasks
jmahlik added a commit to StateFarmIns/sagemaker-python-sdk that referenced this issue Apr 22, 2024
… needed

Moved to using deps to override the test extra being installed in
skip_install tox stages. Now only the needed deps are installed.

Improves install time by around 7x.

Closes aws#4591
liujiaorr pushed a commit that referenced this issue Apr 22, 2024
* change: adjust tox black stages to install from requirements file

Also remove uneeded config in the command, it is now set in the pyproject.toml
so IDE's and other tooling outside tox picks up the setting.

* change: improve tox install times by not installing all deps when not needed

Moved to using deps to override the test extra being installed in
skip_install tox stages. Now only the needed deps are installed.

Improves install time by around 7x.

Closes #4591

* change: bump twine version to 5.0.0

Twine 3.8.0 didn't include its dependencies. The tox stage happened to
work from packaging being installed as a transitive.

Ref: pypa/twine#894
malav-shastri pushed a commit to malav-shastri/sagemaker-python-sdk that referenced this issue Jun 20, 2024
* change: adjust tox black stages to install from requirements file

Also remove uneeded config in the command, it is now set in the pyproject.toml
so IDE's and other tooling outside tox picks up the setting.

* change: improve tox install times by not installing all deps when not needed

Moved to using deps to override the test extra being installed in
skip_install tox stages. Now only the needed deps are installed.

Improves install time by around 7x.

Closes aws#4591

* change: bump twine version to 5.0.0

Twine 3.8.0 didn't include its dependencies. The tox stage happened to
work from packaging being installed as a transitive.

Ref: pypa/twine#894
jiapinw pushed a commit to jiapinw/sagemaker-python-sdk that referenced this issue Jun 25, 2024
* change: adjust tox black stages to install from requirements file

Also remove uneeded config in the command, it is now set in the pyproject.toml
so IDE's and other tooling outside tox picks up the setting.

* change: improve tox install times by not installing all deps when not needed

Moved to using deps to override the test extra being installed in
skip_install tox stages. Now only the needed deps are installed.

Improves install time by around 7x.

Closes aws#4591

* change: bump twine version to 5.0.0

Twine 3.8.0 didn't include its dependencies. The tox stage happened to
work from packaging being installed as a transitive.

Ref: pypa/twine#894
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants