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

Call echo_logs only when in DEBUG mode #167

Merged
merged 6 commits into from
Feb 23, 2023
Merged

Call echo_logs only when in DEBUG mode #167

merged 6 commits into from
Feb 23, 2023

Conversation

danielhollas
Copy link
Contributor

@danielhollas danielhollas commented Jan 30, 2023

The echo_logs function prints out the logs from the container (i.e. the output of aiidalab-launch logs) while the user waits for the container to start. However, logs are printed with logging.DEBUG so there's no point in calling this function unless this level of logging is enabled (via the -vvv cmdline parameter).

We have some indication that this function is problematic in #154 so calling it only when neccessary might help. We also have issues with flaky tests, #163. While the change here does not solve the flakiness (see failures here and here), it does seem to help since the test suite no longer times out, but actually fails in a reasonable amount of time. I haven't been able to pinpoint the reason for the failures, looking at the error messages it seems that the container fails to start randomly. So more investigation is needed but this seems to be a step in right direction at least.

@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Base: 86.72% // Head: 86.54% // Decreases project coverage by -0.18% ⚠️

Coverage data is based on head (3ba48f8) compared to base (e86a331).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
- Coverage   86.72%   86.54%   -0.18%     
==========================================
  Files           9        9              
  Lines         904      907       +3     
==========================================
+ Hits          784      785       +1     
- Misses        120      122       +2     
Flag Coverage Δ
py-3.10 86.43% <100.00%> (-0.18%) ⬇️
py-3.8 86.39% <100.00%> (+0.04%) ⬆️
py-3.9 86.50% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiidalab_launch/__main__.py 80.12% <100.00%> (+0.19%) ⬆️
aiidalab_launch/util.py 84.37% <0.00%> (-1.57%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@danielhollas danielhollas changed the title WIP: Trying to fix flaky tests Call echo_logs only when in DEBUG mode Feb 1, 2023
@danielhollas danielhollas self-assigned this Feb 1, 2023
@danielhollas danielhollas marked this pull request as ready for review February 1, 2023 15:27
Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

This looks all good to me! Thanks!

@unkcpz
Copy link
Member

unkcpz commented Feb 23, 2023

I am going to release it after this one is merged.

@danielhollas
Copy link
Contributor Author

@unkcpz thanks. Feel free to merge, I do not have permissions.

@unkcpz unkcpz merged commit addf493 into main Feb 23, 2023
@unkcpz unkcpz deleted the flaky-tests-again branch February 23, 2023 10:04
@unkcpz
Copy link
Member

unkcpz commented Feb 23, 2023

@danielhollas merged thanks!

I do not have permissions.

Let me know if you need higher permission for convenient management.

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.

None yet

2 participants