Skip to content

Fix timeouts related to metalearnings tests #1508

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

Merged
merged 9 commits into from
Jun 17, 2022

Conversation

eddiebergman
Copy link
Contributor

Adding debug statements for now to find the problem on the github action servers as it cleanly finished locally.

Closes #1506

@eddiebergman eddiebergman added the maintenance Internal maintenance label Jun 10, 2022
@eddiebergman eddiebergman added this to the V0.15 milestone Jun 10, 2022
@eddiebergman eddiebergman self-assigned this Jun 10, 2022
@eddiebergman eddiebergman changed the base branch from master to development June 10, 2022 15:43
@codecov
Copy link

codecov bot commented Jun 14, 2022

Codecov Report

Merging #1508 (879e2df) into development (e9c0318) will increase coverage by 0.06%.
The diff coverage is 86.76%.

@@               Coverage Diff               @@
##           development    #1508      +/-   ##
===============================================
+ Coverage        83.77%   83.83%   +0.06%     
===============================================
  Files              152      153       +1     
  Lines            11667    11694      +27     
  Branches          2037     2047      +10     
===============================================
+ Hits              9774     9804      +30     
+ Misses            1344     1339       -5     
- Partials           549      551       +2     

Impacted file tree graph

@eddiebergman eddiebergman requested a review from mfeurer June 16, 2022 11:57
@eddiebergman
Copy link
Contributor Author

eddiebergman commented Jun 16, 2022

I switched to using import tempfile and using gettempdir() instead of explicitly using /tmp. It seemed to not trigger as a result. Maybe the github action servers have some special folder for it and the stochastic nature of the problem was due to node allocation and usage.

I'm going to run the actions a few time and see if it comes up again.

@eddiebergman
Copy link
Contributor Author

All the timeouts that occur seem to have moved to the next script where there was a hardcoded /tmp again. Renamed it and hopefully it solves it.

Copy link
Contributor

@mfeurer mfeurer left a comment

Choose a reason for hiding this comment

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

Looks good to me. Does it solve all issues?

@eddiebergman
Copy link
Contributor Author

I just set explicit timeouts for each step and bumped the overall timeout per test from 300s to 600s. If one of them actually does hang then at least we'll get a debug message in the future

@eddiebergman
Copy link
Contributor Author

I removed the comment you left review on, if all the tests pass then I think that can be merged

@mfeurer mfeurer merged commit 9d63cb5 into development Jun 17, 2022
@mfeurer mfeurer deleted the fix-timeouts-related-to-metalearnings-tests branch June 17, 2022 13:26
eddiebergman added a commit that referenced this pull request Aug 18, 2022
* Add debug statements and 30s timeouts

* Fix formatting

* Update internal timeout param

* +timeout, use allocated tmpdir

* +timeout, use allocated tmpdir

* Remove another occurence of explicit `tmp`

* Increase timelimits once again

* Remove incomplete comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Internal maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix timeouts related to metalearnings tests
2 participants