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.

Sign up for GitHub

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

Jump to bottom

feat: introduce new flag to avoid using pip freeze and pip show #106

Merged
merged 6 commits into from
May 16, 2024
Merged

feat: introduce new flag to avoid using pip freeze and pip show #106

merged 6 commits into from
May 16, 2024

Conversation

JudeNiroshan
Copy link
Contributor

Description

Improve Python build tree Performance in EXHORT-JAVA-API
JIRA: https://issues.redhat.com/browse/APPENG-2154

Related issue (if any): fixes #issue_number_goes_here
same fix has been done in exhort-javascript-api -> RHEcosystemAppEng/exhort-javascript-api#121

Checklist

  • I have followed this repository's contributing guidelines.
  • I will adhere to the project's code of conduct.

Additional information

Anything else?

@JudeNiroshan JudeNiroshan force-pushed the include-pipdeptree branch 2 times, most recently from 8244dd6 to 33750c1 Compare May 7, 2024 12:47
@JudeNiroshan
feat: introduce new flag to avoid using pip freeze and pip show
efbba2e 
Signed-off-by: Jude Niroshan <jude.niroshan11@gmail.com>
@JudeNiroshan JudeNiroshan marked this pull request as ready for review May 7, 2024 12:52
zvigrinberg
zvigrinberg requested changes May 7, 2024
View reviewed changes
Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

Hi @JudeNiroshan
In General good job, but please see my comments, there are few minor issues and recommendation, so you need couple of slight changes.

src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Outdated Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Outdated Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Outdated Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Outdated Show resolved Hide resolved
src/test/java/com/redhat/exhort/providers/Python_Provider_Test.java Show resolved Hide resolved
@JudeNiroshan JudeNiroshan force-pushed the include-pipdeptree branch 2 times, most recently from b2ae067 to a1ea344 Compare May 8, 2024 10:31
@JudeNiroshan
refactor: use java streams
d572340 
Signed-off-by: Jude Niroshan <jude.niroshan11@gmail.com>
@JudeNiroshan
refactor: rename variable to targetDeps
2f6e193 
Signed-off-by: Jude Niroshan <jude.niroshan11@gmail.com>
@JudeNiroshan
refactor: remove unused block
421aee9 
Signed-off-by: Jude Niroshan <jude.niroshan11@gmail.com>
@JudeNiroshan
Copy link
Contributor Author

Here are the manual test results for a requirements.txt which has over 50 dependencies.

`python --version` == 3.10.8
	Without pipdepstree
		- 5175 miliseconds
	With pipdeptree
		- 2528 milliseconds

`python --version` == 3.11.8
	Without pipdeptree
		- 2685 miliseconds
	With pipdeptree
		- 2174 miliseconds

`python --version` == 3.12.2
	Without pipdepstree
		- 3647, 2954 miliseconds
	With pipdeptree
		- 3069, 2406 milliseconds

zvigrinberg
zvigrinberg requested changes May 9, 2024
View reviewed changes
Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

Hi @JudeNiroshan
Please see my comments, few minor changes required to get this approved.
Thanks.

src/main/java/com/redhat/exhort/utils/PythonDependency.java Outdated Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonControllerBase.java Outdated Show resolved Hide resolved
src/main/java/com/redhat/exhort/utils/PythonDependency.java Show resolved Hide resolved
@JudeNiroshan
refactor: remove default constructor
b7b2a3a 
Signed-off-by: Jude Niroshan <jude.niroshan11@gmail.com>
@JudeNiroshan
refactor: provide context in runtime error
d34f4a5 
Signed-off-by: Jude Niroshan <jude.niroshan11@gmail.com>
zvigrinberg
zvigrinberg approved these changes May 15, 2024
View reviewed changes
Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

@JudeNiroshan
LGTM - Approved.
Thanks!

@JudeNiroshan JudeNiroshan merged commit ec7a743 into RHEcosystemAppEng:main May 16, 2024
5 checks passed
@JudeNiroshan JudeNiroshan deleted the include-pipdeptree branch May 16, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zvigrinberg zvigrinberg zvigrinberg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JudeNiroshan @zvigrinberg