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

Improve actions time #459

Merged
merged 3 commits into from
May 30, 2021
Merged

Improve actions time #459

merged 3 commits into from
May 30, 2021

Conversation

Hritik14
Copy link
Collaborator

@Hritik14 Hritik14 commented May 20, 2021

Actions are majorly meant to perform tests on our codebase, yet they spend majority of their time on setting up the environment.
Currently this only speeds up the pytest actions. We may also improve other ones after a discussion.

This PR comprises of two commits. The profiling of both commits were performed in Hritik14#2 and Hritik14#3


commit 135c95f (HEAD -> actions)
Author: Hritik Vijay [email protected]
Date: Thu May 20 22:25:42 2021 +0530

Use cache for pip

Earlier Install Dependencies used to take 50-60s α
Now it takes ~10s including cache retrival time β

The total workflow time has reduced from ~1:40 to ~0:55

α: https://github.com/nexB/vulnerablecode/runs/2623306124?check_suite_focus=true
https://github.com/nexB/vulnerablecode/runs/2623290485?check_suite_focus=true
https://github.com/nexB/vulnerablecode/runs/2594778880?check_suite_focus=true

β: https://github.com/Hritik14/vulnerablecode/runs/2632140380?check_suite_focus=true
https://github.com/Hritik14/vulnerablecode/runs/2632128817?check_suite_focus=true
https://github.com/Hritik14/vulnerablecode/runs/2632109016?check_suite_focus=true

Signed-off-by: Hritik Vijay [email protected]


commit 9114cb4
Author: Hritik Vijay [email protected]
Date: Thu May 20 01:06:20 2021 +0530

Eliminate postgresql docker container

Initializing docker containers used to take 23-25s alone. α
Now Initializing postgresql (Setup database) takes 4-6s. β

α: https://github.com/nexB/vulnerablecode/runs/2623306124?check_suite_focus=true https://github.com/nexB/vulnerablecode/runs/2623290485?check_suite_focus=true https://github.com/nexB/vulnerablecode/runs/2594778880?check_suite_focus=true

β: https://github.com/Hritik14/vulnerablecode/runs/2631513782?check_suite_focus=true https://github.com/Hritik14/vulnerablecode/runs/2631540610?check_suite_focus=true https://github.com/Hritik14/vulnerablecode/runs/2631552958?check_suite_focus=true

Signed-off-by: Hritik Vijay [email protected]

@sbs2001
Copy link
Collaborator

sbs2001 commented May 22, 2021

I don't think cache for venv is a good idea, it would need some manual work when deps change right ?

Dropping containers -> good idea

@Hritik14
Copy link
Collaborator Author

Key of the cache depends on requirements.txt and would invalidate whenever dependencies change. So there wouldn't be any manual involvement.

Copy link
Collaborator

@sbs2001 sbs2001 left a comment

Choose a reason for hiding this comment

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

LGTM :)

@sbs2001 sbs2001 merged commit 68d3f32 into aboutcode-org:main May 30, 2021
@Hritik14 Hritik14 mentioned this pull request Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants