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

Unit tests are running slow / CI takes a long time. #84

Closed
alxmrs opened this issue Jan 14, 2022 · 4 comments
Closed

Unit tests are running slow / CI takes a long time. #84

alxmrs opened this issue Jan 14, 2022 · 4 comments
Labels
good first issue Good for newcomers

Comments

@alxmrs
Copy link
Collaborator

alxmrs commented Jan 14, 2022

I observe that our CI takes a long time to complete. It is early in the lifetime of this project for our unit tests to be slow (certainly, the codebase is small enough). Having fast tests and an expedient CI system increases the pace of development & bugfixes.

This bug involves investigating why our unit tests takes a long time to run. This may be related to #53.

@alxmrs alxmrs added the good first issue Good for newcomers label Jan 14, 2022
@CillianFn
Copy link
Contributor

CillianFn commented Jan 18, 2022

@alxmrs This is caused by these two tests and the exponential backoff here

image

I'm thinking of overriding some of the defaults for the tests only, sound good?

Edit: I feel like ValueError shouldn't be triggering retry_filter=retry.retry_on_server_errors_and_timeout_filter

This might be what we're after...

image

@alxmrs
Copy link
Collaborator Author

alxmrs commented Jan 18, 2022

Great investigation! It makes sense that retries are causing the delay.

I looked at Apache Beam's filter definition, and I don't understand why a value error would cause retries. I wonder if the underlying GCS File IO modules are wrapping the exceptions. Does your fix from the edit save time in practice?

@CillianFn
Copy link
Contributor

@alxmrs Seems to be back in business 🐢 🐇
https://github.com/google/weather-tools/runs/4860959961?check_suite_focus=true

@alxmrs
Copy link
Collaborator Author

alxmrs commented Jan 19, 2022

Thanks Cillian! Marking as closed.

@alxmrs alxmrs closed this as completed Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants