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

Allow bypassing the new warning wait period through PIO_ACKWARN env var. #1810

Merged
merged 4 commits into from
Jan 12, 2024

Conversation

SpicyLemon
Copy link
Contributor

@SpicyLemon SpicyLemon commented Jan 12, 2024

Description

This PR primarily adds a check of the PIO_ACKWARN environment variable. If it exists, and isn't something false-like (i.e. "0", "f", '"F", "false", "False", or "FALSE"`), the 30-second sleep is skipped after issuing the config warnings.

This PR also:

  • Tweaks the wrapper a bit primarily to allow the tests to run without actually having to wait on the sleeps.
  • Adds another log message with the sleep that says how to make the sleep go away, and moves the sleep messages to after the warnings.
  • Definition of the default store loader is back in app/app.go#New. This made tests on the wrapper stuff easier since I could always use a mock store loader for them.
  • Provides a standard way to create a logger that writes to a buffer. This makes it easier to test what has been logged. The few places that had such a logger copy/pasted were switched to use this new helper.

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

…ot actually do the sleeps. Also allow bypassing the sleep via env var. Standardize some of the buffered logger stuff too.
@SpicyLemon SpicyLemon requested a review from a team as a code owner January 12, 2024 01:40
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (4d2e4d7) 73.97% compared to head (b08804d) 73.97%.
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1810   +/-   ##
=======================================
  Coverage   73.97%   73.97%           
=======================================
  Files         315      315           
  Lines       44764    44772    +8     
=======================================
+ Hits        33114    33120    +6     
- Misses      10269    10271    +2     
  Partials     1381     1381           
Files Coverage Δ
app/app.go 85.38% <100.00%> (ø)
app/store_loader.go 95.74% <95.45%> (-4.26%) ⬇️

… it (was included in the non-unit-test-only app/test_helpers.go file) but couldn't get it because it wasn't part of accessible stuff.
@SpicyLemon SpicyLemon merged commit ec697ce into main Jan 12, 2024
40 checks passed
@SpicyLemon SpicyLemon deleted the dwedul/bypass-warning-wait branch January 12, 2024 17:25
SpicyLemon added a commit that referenced this pull request Jan 12, 2024
…ar. (#1810)

* Tweak the store loader config warning stuff a bit to make the tests not actually do the sleeps. Also allow bypassing the sleep via env var. Standardize some of the buffered logger stuff too.

* Add changelog entry.

* Fix imports in app/store_loader_test.go.

* Move the buffered logger to internal because the rosetta stuff needed it (was included in the non-unit-test-only app/test_helpers.go file) but couldn't get it because it wasn't part of accessible stuff.
SpicyLemon added a commit that referenced this pull request Jan 12, 2024
…ar. (#1810) (#1811)

* Tweak the store loader config warning stuff a bit to make the tests not actually do the sleeps. Also allow bypassing the sleep via env var. Standardize some of the buffered logger stuff too.

* Add changelog entry.

* Fix imports in app/store_loader_test.go.

* Move the buffered logger to internal because the rosetta stuff needed it (was included in the non-unit-test-only app/test_helpers.go file) but couldn't get it because it wasn't part of accessible stuff.
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.

3 participants