fix ssl verify and update tests #16149
Conversation
github-actions
bot
added
bug
CodSpeed Performance ReportMerging #16149 will not alter performanceComparing Summary
|
|
According to lundberg/respx#277 |
github-actions
bot
added
the
upstream dependency
zzstoatzz
force-pushed
the
fix-ssl-verify-ctx
branch
from
78ecfe6
to
0a6c723
Compare
| uv pip install --upgrade -r requirements-markdown-tests.txt | ||
| uv pip install --upgrade -e '.[dev]' |
There was a problem hiding this comment.
swapped the order here
| if PREFECT_API_TLS_INSECURE_SKIP_VERIFY: | ||
| httpx_settings.setdefault("verify", False) | ||
| # Create an unverified context for insecure connections | ||
| ctx = ssl.create_default_context() | ||
| ctx.check_hostname = False | ||
| ctx.verify_mode = ssl.CERT_NONE | ||
| httpx_settings.setdefault("verify", ctx) | ||
| else: | ||
| cert_file = PREFECT_API_SSL_CERT_FILE.value() | ||
| if not cert_file: | ||
| cert_file = certifi.where() | ||
| httpx_settings.setdefault("verify", cert_file) | ||
| # Create a verified context with the certificate file | ||
| ctx = ssl.create_default_context(cafile=cert_file) | ||
| httpx_settings.setdefault("verify", ctx) |
There was a problem hiding this comment.
You are repeating a few things in both branches that could be simplified out:
ctx = ssl.create_default_context()
if PREFECT_API_TLS_INSECURE_SKIP_VERIFY:
# Update the context to not verify for insecure connections
ctx.check_hostname = False
ctx.verify_mode = ssl.CERT_NONE
else:
cert_file = PREFECT_API_SSL_CERT_FILE.value()
if not cert_file:
cert_file = certifi.where()
# Update the context with the certificate file
ctx.load_verify_locations(cafile=cert_file)
httpx_settings.setdefault("verify", ctx)There was a problem hiding this comment.
hi @mjpieters! you are correct, thanks for pointing this out! in this draft PR i was exploring what fixes would be necessary
There was a problem hiding this comment.
Ah, of course, sorry if I jumped the gun a bit here.
There was a problem hiding this comment.
@mjpieters no worries! if you're interested in following along, we're working through the same upstream issue's impact on our tests here
| if PREFECT_API_TLS_INSECURE_SKIP_VERIFY: | ||
| httpx_settings.setdefault("verify", False) | ||
| # Create an unverified context for insecure connections | ||
| ctx = ssl.create_default_context() | ||
| ctx.check_hostname = False | ||
| ctx.verify_mode = ssl.CERT_NONE | ||
| httpx_settings.setdefault("verify", ctx) | ||
| else: | ||
| cert_file = PREFECT_API_SSL_CERT_FILE.value() | ||
| if not cert_file: | ||
| cert_file = certifi.where() | ||
| httpx_settings.setdefault("verify", cert_file) | ||
| # Create a verified context with the certificate file | ||
| ctx = ssl.create_default_context(cafile=cert_file) | ||
| httpx_settings.setdefault("verify", ctx) |
zzstoatzz
force-pushed
the
fix-ssl-verify-ctx
branch
from
ea03de4
to
64618b6
Compare
|
@desertaxle related to sync discussion, I've opened #16179 |
closes #16148
respxwith the fix until the main version is releasedprefectinstead overwriting itrelatedly, #16168 fixes httpx issues with tests and an issue with the worker's cloud client context when adding flow run labels