feat(python): add TLS and auth args to getting-started examples#2754
feat(python): add TLS and auth args to getting-started examples#2754saie-ch wants to merge 11 commits intoapache:masterfrom
Conversation
saie-ch
force-pushed
the
feature/add-tls-args-to-example
branch
from
a324506 to
861c7ef
Compare
|
Since we use the examples to validate the behaviour of the sdks, my concern is that currently only the "happy path" of the example working without tls is tested. If we are to merge the changes, I'd appreciate it if the example could be tested with the tls config in CI. |
|
I thought about it, and instead of testing tls using the examples, it would be better to test as an integration test. @saie-ch would you be amenable to writing an integration test that will test tls? |
|
@slbotbm Sure, I can write an integration test for TLS. |
foreign/python/pyproject.toml
Outdated
| ] | ||
|
|
||
| [tool.mypy] | ||
| ignore_missing_imports = true |
There was a problem hiding this comment.
what's the reason for this change?
There was a problem hiding this comment.
During the lint job, testcontainers is not available during the mypy check because Docker(uv sync --frozen --extra dev --extra testing) is not installed unless the task is test. This caused mypy to fail with import-not-found.
we can add --extra testing-docker to the lint install step in .github/actions/python-maturin/pre-merge/action.yml and check it once — happy to go with whichever approach you prefer.
There was a problem hiding this comment.
Yes, please update the installation, so the testing-docker is added for both test and lint tasks. If we enable ignoring missing imports, we might miss something in the future.
| await client.connect() | ||
| logger.info("Connected. Logging in user...") | ||
| await client.login_user("iggy", "iggy") | ||
| await client.login_user(args.username, args.password) |
There was a problem hiding this comment.
if the credentials are provided in the connection string, then auto_login is enabled, so calling login is no longer needed as a separate step
foreign/python/tests/test_tls.py
Outdated
| # Connect without TLS to a TLS-enabled server | ||
| # IggyClient constructor requires IP address, not hostname | ||
| client = IggyClient(f"127.0.0.1:{port}") | ||
| await client.connect() |
There was a problem hiding this comment.
In this case, doesn't connect already throw an error?
Also, I think the client set up should be as close as possible (we just want to test the difference tls/no tls), so you should use connection string, but without the tls part.
foreign/python/pyproject.toml
Outdated
| ] | ||
|
|
||
| [tool.mypy] | ||
| ignore_missing_imports = true |
There was a problem hiding this comment.
Yes, please update the installation, so the testing-docker is added for both test and lint tasks. If we enable ignoring missing imports, we might miss something in the future.
…l testing-docker for lint
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2754 +/- ##
============================================
- Coverage 67.83% 67.81% -0.02%
Complexity 739 739
============================================
Files 1052 1052
Lines 84635 84635
Branches 61212 61222 +10
============================================
- Hits 57410 57399 -11
+ Misses 24854 24853 -1
- Partials 2371 2383 +12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
5 participants
The Python getting-started examples connection settings has no way to configure TLS, authentication credentials.
What changed?
The producer and consumer examples only supported a basic --tcp-server-address argument.
Both examples now accept --tls, --tls-ca-file, --username, and --password arguments, and use IggyClient.from_connection_string() to build a connection string with optional TLS parameters.
Local Execution : Passed
Pre-commit hooks ran
Tested locally against iggy server on 127.0.0.1:8090 without TLS:
python examples/python/getting-started/producer.py --tcp-server-address 127.0.0.1:8090
python examples/python/getting-started/consumer.py --tcp-server-address 127.0.0.1:8090
Producer sent 5 batches of 10 messages (50 total), consumer consumed all 50 messages successfully.
Tested on manged server with --tls , --tls-ca-file, username, password.
examples/python/getting-started/producer.py
--tls
--tcp-server-address xxxxxxxx
--username xxxx
--password xxxxxx
--tls-ca-file xxxxx
Python SDK tests: 18/18 passed