Enable hot reload by default #6940
Conversation
AndrewChubatiuk
force-pushed
the
enable-hot-reload-by-default
branch
from
ef349fe
to
a60a3f7
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6940 +/- ##
=======================================
Coverage 63.76% 63.76%
=======================================
Files 161 161
Lines 13085 13085
Branches 1812 1812
=======================================
Hits 8344 8344
Misses 4438 4438
Partials 303 303 |
|
@eradman could you please review? |
bin/docker-entrypoint
Outdated
| -w${REDASH_WEB_WORKERS:-4} redash.wsgi:app \ | ||
| --max-requests $MAX_REQUESTS \ | ||
| --max-requests-jitter $MAX_REQUESTS_JITTER \ | ||
| --timeout $TIMEOUT ${args[@]} |
There was a problem hiding this comment.
Let's make "$args" a simple string instead of a BASH array
There was a problem hiding this comment.
moved --reload arg to compose file instead and added ability to pass command extra args to server process, which can be useful for helm chart as well
|
This did not work for me. After starting with |
|
@eradman please set |
AndrewChubatiuk
force-pushed
the
enable-hot-reload-by-default
branch
from
9e0500d
to
6ba8714
Compare
|
@eradman modified makefile to add |
|
Interesting, the concept of the PR seems useful. |
|
@justinclift could you check PR? almost impossible to get any feedback from @eradman |
| @@ -4,6 +4,10 @@ export COMPOSE_DOCKER_CLI_BUILD=1 | |||
| export DOCKER_BUILDKIT=1 | |||
| export COMPOSE_PROFILES=local | |||
|
|
|||
| define random | |||
| $(shell cat /dev/urandom | LC_ALL=C tr -dc 'a-zA-Z0-9' | fold -w 32 | head -n 1) | |||
There was a problem hiding this comment.
Why the change from pwgen, and why is it in this PR?
I'm pretty sure we've had the conversation before about not including extra things in PRs, as it slows down the review process. 😦
There was a problem hiding this comment.
pwgen is not a default tool, this approach should work out of box on linux and on mac
without this change i cannot test locally
There was a problem hiding this comment.
Interesting. Well, it does seem to work ok, and the quality of randomness doesn't really make a difference for development.
The pwgen change should really be in its own PR though, as it's a distinct thing.
What type of PR is this?
Enabled hot reload by default
Description
How is this tested?
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)