Fix handling of SYNAPSE_NO_TLS in docker image#5005
Fix handling of SYNAPSE_NO_TLS in docker image#5005richvdh merged 13 commits intomatrix-org:developfrom f35f0ef9d0e827dae86552d3899f78fc:fix4663
Conversation
|
Signed-off-by: Gabriel Eckerson 21157384+f35f0ef9d0e827dae86552d3899f78fc@users.noreply.github.com |
Codecov Report
@@ Coverage Diff @@
## develop #5005 +/- ##
===========================================
- Coverage 60.7% 60.57% -0.13%
===========================================
Files 332 331 -1
Lines 34172 34213 +41
Branches 5633 5655 +22
===========================================
- Hits 20743 20725 -18
- Misses 11958 12013 +55
- Partials 1471 1475 +4 |
Codecov Report
@@ Coverage Diff @@
## develop #5005 +/- ##
===========================================
- Coverage 60.7% 60.57% -0.14%
===========================================
Files 332 331 -1
Lines 34172 34213 +41
Branches 5633 5655 +22
===========================================
- Hits 20743 20723 -20
- Misses 11958 12015 +57
- Partials 1471 1475 +4 |
|
For link: #4663 |
richvdh
left a comment
There was a problem hiding this comment.
lgtm! thanks. I wrote a couple of thoughts below which I hope you might find useful for future reference.
Another thought is that it would have been nice to factor out some of the logic to a separate function, so that we could use it for other boolean parameters like SYNAPSE_ALLOW_GUEST and SYNAPSE_REPORT_STATS. We can make that change another time though.
|
|
||
| # Convert SYNAPSE_NO_TLS to boolean if exists | ||
| if "SYNAPSE_NO_TLS" in environ: | ||
| tlsanswerstring = str.lower(environ["SYNAPSE_NO_TLS"]) |
There was a problem hiding this comment.
might have been nice to write tlsanswerstring = str.lower(environ.get("SYNAPSE_NO_TLS", "false")) here, to avoid the if condition above.
| if tlsanswerstring in ("true", "on", "1", "yes"): | ||
| environ["SYNAPSE_NO_TLS"] = True | ||
| else: | ||
| if tlsanswerstring in ("false", "off", "0", "no"): |
There was a problem hiding this comment.
would have been nice to use elif rather than another layer of nesting.
|
test failures are due to #5096 |
* develop: (34 commits) Add a default .m.rule.tombstone push rule (#4867) Fix infinite loop in presence handler changelog more logging improvements remove extraneous exception logging Clarify logging when PDU signature checking fails Changelog Add --no-pep-517 to README instructions set PIP_USE_PEP517 = False for tests Fix handling of SYNAPSE_NO_TLS in docker image (#5005) Config option for verifying federation certificates (MSC 1711) (#4967) Remove log error for .well-known/matrix/client (#4972) Prevent "producer not unregistered" message (#5009) add gpg key fingerprint Don't crash on lack of expiry templates Update debian install docs for new key and repo (#5074) Add management endpoints for account validity Send out emails with links to extend an account's validity period Make sure we're not registering the same 3pid twice Newsfile ...
Pull Request Checklist