Use async io where possible to improve runtime performance#163
Use async io where possible to improve runtime performance#163pombredanne merged 18 commits intoaboutcode-org:mainfrom
Conversation
|
@netomi thanks! please check the errors in the CI and also do not forget to regen the tests just in case dependencies in tests have been updated https://github.com/nexB/python-inspector#testing |
|
There are a couple of things that I still need to fix, but the failing CI runs are unrelated to this PR afaict. They also exist for other PRs and look like that the setup is broken. |
|
ok so there are some unit test failure that I need to address. Most of them are from the fact that python-inspector returned duplicate package entries in its result up to now, which I fixed, so the expected test results need to be adjusted to take this change into account. |
|
I could fix most tests (weird thing is that 4 tests fail locally when run in pycharm, but work on the command line so that might be a pycharm problem), there are some weird test failures on macos and windows only. Do you have any idea about that? |
|
still need to take a look at etc/scripts if the there also need to be adjustments after the changes. |
|
@netomi BTW, do not bother with the etc/scripts ... these come from the shared skeleton repo at https://github.com/nexB/skeleton and should not be of concern for you. |
|
Can this be moved forward, please? 😬 |
|
@sschuberth tests are falling, tests needs to be regen |
Would you have time to do this @netomi? |
|
hmm I can take a look, but I am quite discouraged from contributing to a repo that has to constantly rebuilding its tests and they are failing most of the time when creating a PR. You still need to inspect if its related to your changes or the test data is just out of date. |
@netomi I agree this is not great. What do you think we could do as an alternative? |
|
please dont actually call live servers during tests but rather mock their responses with data you might have retrieved from the server at one point in time. I understand that is a bit harder to do, but ultimately results in much more stable and reliable tests imho. PyGitHub has a nice system for their tests where they replay data: |
|
At least one of the errors seems to be
|
|
no idea where this error is coming from. This decorator is not used anywhere in the code. Probably from a dependency? This error btw also occurs for the default branch, so its not related to this PR afaict. |
@TG1999 or @chinyeungli, can you assist here by fixing the test baseline in |
|
Release https://github.com/aboutcode-org/python-inspector/releases/tag/v0.13.1 says to fix the CI issues. So @netomi could you please rebase and give this a try again? |
I guess you should rebase instead to not make the DCO check fail on the merge commit? |
|
so I rebased my PR but there are some test failures after that due to duplicate data in the test data json files. Here is an example: https://github.com/aboutcode-org/python-inspector/blob/main/tests/data/test-api-expected.json |
@TG1999 is that something you could fix in |
|
I could figure out how to regenerate test fixtures. |
|
I am lost, running the tests pass locally. |
|
@netomi I am checking this |
|
@netomi I have pushed a regen fix, let's see how it goes. Additionally sign-off is missing from commits. Please check that. Thanks! |
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com> Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
|
@netomi For Python 3.12 build is failing |
|
ah that is due to requirements, I did test something locally diff --git a/requirements.txt b/requirements.txt
index 91fd48a..f64e888 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -10,7 +10,7 @@ idna==3.3
importlib-metadata==4.12.0
intbitset==3.1.0
packageurl-python==0.10.0
-packaging==21.3
+packaging==24.2
packvers==21.5
pip-requirements-parser==32.0.1
pkginfo2==30.0.0
@@ -24,5 +24,5 @@ text-unidecode==1.3
toml==0.10.2
urllib3==1.26.11
zipp==3.8.1
-aiohttp==3.8.6
+aiohttp==3.11.14
aiofiles==23.2.1
tn@proteus:~/workspace/netomi/python-that solved it |
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
|
why is the signoff needed actually? This is just annoying. Or I am going to enable that by default in my gitconfig. |
|
wow everything passes. |
For the DCO check. That's pretty much standard.
I'm doing the same with a commit template. |
|
Thats the first project that requires my commits to be signed-off. |
I doubt that, ORT does the same. Are you maybe confusing sign-off with signing commits? |
|
I understand the purpose of having my commits signed-off, its a lightweight version of an ICLA. I might have signed-off some commits in the past, but cant recall it anymore tbh. |
@TG1999 or @pombredanne, are we good to merge? |
|
@netomi would you have some benchmark that shows a performance improvement BTW? if yes do you mind to paste this here? |
|
with a simple requirements.txt I get the following numbers: latest release: asyncio version: with the asyncio version you get some nice output with |
|
btw. python-inspector uses distutils internally, which has been removed in python 3.12. So you cant use it anymore with a more recent python version. |
|
this is what I get when installing python-inspector using pipx with python 3.12: |
|
that answer explains why it works in a development environment: https://stackoverflow.com/a/77284076 distutils seems to be included in setuptools. |
This PR fixes #162 .
Things that this PR will modify: