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

Lint is very slow #2419

Closed
ocelotl opened this issue Apr 15, 2024 · 4 comments · Fixed by #2467
Closed

Lint is very slow #2419

ocelotl opened this issue Apr 15, 2024 · 4 comments · Fixed by #2467
Assignees

Comments

@ocelotl
Copy link
Contributor

ocelotl commented Apr 15, 2024

This cannot be applied to this repo because there are many packages that share the same dependency but require different versions of it.

We should instead separate linting in as many actions as packages we have. This will speed up linting in CI significantly thanks to the fact that many jobs can run in parallel.

@aabmass
Copy link
Member

aabmass commented Apr 15, 2024

This cannot be applied to this repo because there are many packages that share the same dependency but require different versions of it.

Can you give an example? I'm also confused how running individual pip install commands would fix that issue if it's the case.

@ocelotl
Copy link
Contributor Author

ocelotl commented Apr 16, 2024

This cannot be applied to this repo because there are many packages that share the same dependency but require different versions of it.

Can you give an example? I'm also confused how running individual pip install commands would fix that issue if it's the case.

Can you give an example? I'm also confused how running individual pip install commands would fix that issue if it's the case.

Just to be clear, I don't want to have multiple pip install commands, at all, it is horribly slow. I want a solution for this problem.

Actually having multiple pip install commands "fixes" (in the worst sense of the word) this problem because this is what happens with multiple pip install commands:

(django) tigre@hilleman:~/sandbox/django$ pip install django==1.1.3
Collecting django==1.1.3
  Downloading Django-1.1.3.tar.gz (5.7 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 5.7/5.7 MB 4.6 MB/s eta 0:00:00
  Preparing metadata (setup.py) ... done
Building wheels for collected packages: django
  Building wheel for django (setup.py) ... done
  Created wheel for django: filename=Django-1.1.3-py3-none-any.whl size=4347255 sha256=e9fe2d43fd7af7e2e8777636a43213a91bdf873367c31979b3dfee9423aaee27
  Stored in directory: /home/tigre/.cache/pip/wheels/47/dc/77/e799a0fa6f184ec1ff4b40b74810ae5d0a22ab9de5694867b3
Successfully built django
Installing collected packages: django
Successfully installed django-1.1.3
(django) tigre@hilleman:~/sandbox/django$ pip install django==4.2.1
Collecting django==4.2.1
  Downloading Django-4.2.1-py3-none-any.whl.metadata (4.1 kB)
Collecting asgiref<4,>=3.6.0 (from django==4.2.1)
  Using cached asgiref-3.8.1-py3-none-any.whl.metadata (9.3 kB)
Collecting sqlparse>=0.3.1 (from django==4.2.1)
  Downloading sqlparse-0.5.0-py3-none-any.whl.metadata (3.9 kB)
Downloading Django-4.2.1-py3-none-any.whl (8.0 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 8.0/8.0 MB 9.8 MB/s eta 0:00:00
Using cached asgiref-3.8.1-py3-none-any.whl (23 kB)
Downloading sqlparse-0.5.0-py3-none-any.whl (43 kB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 44.0/44.0 kB 6.3 MB/s eta 0:00:00
Installing collected packages: sqlparse, asgiref, django
  Attempting uninstall: django
    Found existing installation: Django 1.1.3
    Uninstalling Django-1.1.3:
      Successfully uninstalled Django-1.1.3
Successfully installed asgiref-3.8.1 django-4.2.1 sqlparse-0.5.0

The previously installed version gets uninstalled, resulting in a usable virtual environment, this gives the impression that using multiple pip install commands "fixes" dependency conflicts.

Let's check out this commit and run tox -e lint. It fails with:

ERROR: Cannot install urllib3==1.26.18 and urllib3==2.2.1 because these package versions have conflicting dependencies.                                                                                                                                                                                                                                                                     The conflict is caused by:                                                                                                                                                                        The user requested urllib3==1.26.18                                                                                                                                                           The user requested urllib3==2.2.1                                                                                                                                                                                                                                                                                                                                                       To fix this you could try to:                                                                                                                                                                 1. loosen the range of package versions you've specified                                                                                                                                      2. remove package versions to allow pip attempt to solve the dependency conflict

We could try and change the version of urllib3 in some of the test-requirements*.txt files to make it the same (which is pretty much what I did here for protobuf:

diff --git a/instrumentation/opentelemetry-instrumentation-boto/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-boto/test-requirements.txt
index b8097cb4..642dbed8 100644
--- a/instrumentation/opentelemetry-instrumentation-boto/test-requirements.txt
+++ b/instrumentation/opentelemetry-instrumentation-boto/test-requirements.txt
@@ -62,7 +62,7 @@ sshpubkeys==3.3.1
 sympy==1.12
 tomli==2.0.1
 typing_extensions==4.10.0
-urllib3==1.26.18
+urllib3==2.2.1
 Werkzeug==3.0.1
 wrapt==1.16.0
 xmltodict==0.13.0
diff --git a/instrumentation/opentelemetry-instrumentation-boto3sqs/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-boto3sqs/test-requirements.txt
index 0358844f..0effff9e 100644
--- a/instrumentation/opentelemetry-instrumentation-boto3sqs/test-requirements.txt
+++ b/instrumentation/opentelemetry-instrumentation-boto3sqs/test-requirements.txt
@@ -17,7 +17,7 @@ s3transfer==0.10.0
 six==1.16.0
 tomli==2.0.1
 typing_extensions==4.10.0
-urllib3==1.26.18
+urllib3==2.2.1
 wrapt==1.16.0
 zipp==3.17.0
 -e opentelemetry-instrumentation
diff --git a/instrumentation/opentelemetry-instrumentation-botocore/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-botocore/test-requirements.txt
index e12d6f6b..23443d99 100644
--- a/instrumentation/opentelemetry-instrumentation-botocore/test-requirements.txt
+++ b/instrumentation/opentelemetry-instrumentation-botocore/test-requirements.txt
@@ -60,7 +60,7 @@ sshpubkeys==3.3.1
 sympy==1.12
 tomli==2.0.1
 typing_extensions==4.10.0
-urllib3==1.26.18
+urllib3==2.2.1
 Werkzeug==2.1.2
 wrapt==1.16.0
 xmltodict==0.13.0
diff --git a/instrumentation/opentelemetry-instrumentation-elasticsearch/test-requirements-0.txt b/instrumentation/opentelemetry-instrumentation-elasticsearch/test-requirements-0.txt
index 216d1c0b..43c896f6 100644
--- a/instrumentation/opentelemetry-instrumentation-elasticsearch/test-requirements-0.txt
+++ b/instrumentation/opentelemetry-instrumentation-elasticsearch/test-requirements-0.txt
@@ -15,7 +15,7 @@ python-dateutil==2.8.2
 six==1.16.0
 tomli==2.0.1
 typing_extensions==4.10.0
-urllib3==1.26.18
+urllib3==2.2.1
 wrapt==1.16.0
 zipp==3.17.0
 -e opentelemetry-instrumentation
diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/test-requirements-0.txt b/instrumentation/opentelemetry-instrumentation-urllib3/test-requirements-0.txt
index 730ef169..1f10502f 100644
--- a/instrumentation/opentelemetry-instrumentation-urllib3/test-requirements-0.txt
+++ b/instrumentation/opentelemetry-instrumentation-urllib3/test-requirements-0.txt
@@ -12,7 +12,7 @@ pytest==7.1.3
 pytest-benchmark==4.0.0
 tomli==2.0.1
 typing_extensions==4.10.0
-urllib3==1.26.18
+urllib3==2.2.1
 wrapt==1.16.0
 zipp==3.17.0
 -e opentelemetry-instrumentation

boto tests (and other tests too) fail with these changes.

After thinking a bit about this issue, I now think that there is no reason why there should exist a combination of package versions that should satisfy all tests for all packages including lint. We managed to find one for the core repo but it was probably just luck.

I now think the solution for this issue is to separate lint into several different jobs, one for each package, so that we don't have these dependency issues.

@aabmass
Copy link
Member

aabmass commented Apr 16, 2024

@ocelotl if we remove the transitive dependencies from the test-requirements.txt files, would it fix the issue?

I now think the solution for this issue is to separate lint into several different jobs, one for each package, so that we don't have these dependency issues.

I agree this would be better in theory, but I'm worried this will be slow.

@ocelotl
Copy link
Contributor Author

ocelotl commented Apr 16, 2024

@ocelotl if we remove the transitive dependencies from the test-requirements.txt files, would it fix the issue?

I now think the solution for this issue is to separate lint into several different jobs, one for each package, so that we don't have these dependency issues.

I agree this would be better in theory, but I'm worried this will be slow.

It means installing the same packages several times for every package (which is bad). It would mean we can use Github CI paralellization (which is good) and it would also mean users could run lint only for a particular package in their local computers (which is also good since I imagine most changes involve only one package).

There is only one way to find out, I'll try to separate lint into multiple actions and we'll see what happens.

@ocelotl ocelotl changed the title Use a single install command for lint Lint is very slow Apr 19, 2024
@ocelotl ocelotl changed the title Lint is very slow Do not install unnecessary packages Apr 19, 2024
@ocelotl ocelotl changed the title Do not install unnecessary packages Lint is very slow Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants