feat(backend-python): add basic telestion library for python by cb0s · Pull Request #423 · wuespace/telestion

cb0s · 2024-03-13T20:18:15Z

Summary

Add Telestion libraries for creating backend libraries in Python.

Details

The provided tools should - like all other backends - provide necessary tooling for parsing and combining various config methods and start the nats client.

Additional information

For testing the backend, a vitual environment with Python 3.12 is recommended (earlier versions might not work). The necessary requirements can be downloaded from the requirements.txt.

CLA

I have signed the individual contributor's license agreement and sent it to the board of the WüSpace e. V. organization.

main
- feat(backend-python): add basic telestion library for python #423 👈

… fix various bugs, add test script for development (will be deleted before merge into main)

…tting up the health check in designated function

…d add basic doc strings

pklaschka · 2024-03-15T16:57:25Z

Please also adjust the .github/CODEOWNERS before we merge this – I would rather not own the Python library, considering that the last time I did major work in Python was over two years ago 🙈.

cb0s · 2024-03-16T16:39:41Z

Please also adjust the .github/CODEOWNERS before we merge this – I would rather not own the Python library, considering that the last time I did major work in Python was over two years ago 🙈.

We already set this up when we committed Add a CODEOWNERS file.

pklaschka · 2024-03-17T12:34:57Z

Please also adjust the .github/CODEOWNERS before we merge this – I would rather not own the Python library, considering that the last time I did major work in Python was over two years ago 🙈.

We already set this up when we committed Add a CODEOWNERS file.

Interesting... For some reason, GitHub seems to believe I'm the owner 🤔

pklaschka · 2024-03-17T12:35:50Z

Ah, wait... It's only because it includes documentation (the README.md) – disregard what I said 🙈

pklaschka

Preliminary review – see inline comments.

pklaschka · 2024-03-22T12:46:18Z

+def build_config(clazz: type[_TelestionConfigT] = None) -> _TelestionConfigT:
+    if clazz is None:
+        clazz = TelestionConfig


Why default to None instead of TelestionConfig if you re-set it immediately afterwards?

Because in Python function default arguments are retained between executions. Yes, that's a feature, not a bug.

https://docs.python.org/3/faq/programming.html#why-are-default-values-shared-between-objects

pklaschka · 2024-03-22T13:32:06Z

+    if '://' in url:
+        _, url = url.split('://', 1)
+
+    return f"nats://{config.username}:{config.password}@{url}"


Breaks if the username contains a colon (e.g., database:backup)

pklaschka · 2024-03-22T13:34:44Z

+    return json.loads(msg, **loads_kwargs)
+
+
+def _prepare_nats_url(config: TelestionConfig) -> str:


Overall, I think we should look at pre-existing URL handling APIs (may that be in the Python core or a library), due to the numerous potential attack vectors currently present in the code.

Git tries to mimic CRLF files on Windows by changing the line endings on checkout and silently removes them on commit. TLDR: On Windows, please set: ``` git config --global core.autocrlf true ``` see https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings

… test behaviour

Co-authored-by: Zuri Klaschka <pklaschka@users.noreply.github.com>

cb0s · 2024-12-23T00:07:27Z

One of the behaviour tests is still failing, trying to fix that tomorrow.
I would love to get some general feedback though that we can finally merge this tomorrow as an early X-Mas present to us.

…custom arguments in `build_config()` and fix tests

fussel178

Preliminary review, I currently don't have the time to further look into the config.py. Separate review is following.

Good first implementation. I've added my comments below.

Some things we need to consider:

we need to write some documentation with example usages to get a feeling how we want to use the library
deployment: maybe add a pyproject.toml specified in PEP518 and if it works with our special environment (project root ≠ git root)

fussel178 · 2025-01-08T15:25:33Z

+    # ])
+    # asyncio.run(main())
+    loop = asyncio.get_event_loop()
+    loop.run_until_complete(asyncio.wait_for(main(), 1))    # fail automatically after one second


I think, 1 second is a little bit short especially on a lower end test machine.
I would generously increase this to 10 seconds.
And maybe extract it into global constant, e.g. RUN_TIMEOUT or so.

Unrelated question:
Should we add an execution timeout to the container.run command in backend-features/lib/testbed.py?

telestion/backend-features/lib/testbed.py

Lines 57 to 63 in 2993291

result = context.docker.containers.run(

context.testbed,

cmd,

environment=env,

auto_remove=True,

network=nats_network_name(context),

).splitlines()[-1].decode()

fussel178 · 2025-01-08T15:27:38Z

+        nats_connection_options = {
+            "allow_reconnect": False,       # only try to connect once
+            "max_reconnect_attempts": 0,    # test
+            "connect_timeout": 1,           # fail after 1 sec.


Maybe extract it to global constant? (e.g. NATS_CONNECT_TIMEOUT)

fussel178 · 2025-01-08T15:31:56Z

With our new E2E test I think we don't this folder anymore. 😉

fussel178 · 2025-01-08T15:40:21Z

Can you put the test and some other example usages in a separate folder samples?

Maybe structured like the backend-go or backend-deno projects?

https://github.com/wuespace/telestion/tree/2993291752bd85d6b474c61947316be264a5765f/backend-deno

https://github.com/wuespace/telestion/tree/2993291752bd85d6b474c61947316be264a5765f/backend-go

fussel178 · 2025-01-08T16:23:40Z

+    if config.nats_user is None or config.nats_password is None:
+        return url
+
+    if '://' in url:
+        _, url = url.split('://', 1)
+
+    return f"nats://{config.nats_user}:{config.nats_password}@{url}"


To prevent URL injection and other stuff can you use the urllib.parse.urlparse function to parse the nats_url input?
Then, create another urllib.parse.ParseResult object by copying over the important parts from urllib.parse.urlparse and inserting username and password here.
Finally, you can return a (hopefully) valid URL with urllib.parse.ParseResult.geturl().

Notice:

The urllib.parse function does not validate the user input!
Maybe we need to add this to the doc comments somewhere...

cf. https://docs.python.org/3/library/urllib.parse.html

I don't really understand why you would need to validate the URL here. Isn't it only used in configuring a new nats microservice from config or ENV_VARIABLES? This should definitely be a very controlled environment, thus I didn't really consider it necessary when I wrote it originally.
Also, if urllib doesn't check it anyway and therefore doesn't really bring us new features, I would advocate against using it and keep the f-string here. What do you think?

fussel178 · 2025-01-08T18:30:59Z

I'm thinking if it would be nice to export the start_service function explicitly. 🤔

So you don't need to import start_service from telestion.backend.lib but from telestion.

fussel178 · 2025-01-08T18:32:07Z

Can you move the telestion folder with the library code from src into the project root?

The nats.py lib does it, too. (https://github.com/nats-io/nats.py)

Yes of course! I just wanted to stay consistent with the go and typescript backend. Tbh. I would like it better that way anyways :D

Revisiting the code of the other libraries - either we changed it already or I had another reason for it that I don't remember at the moment. It should be possible though.

fussel178 · 2025-01-08T18:38:46Z

+# Telestion Python Backend
+
+This repository contains everything to build Telestion backend services in Python.
+Additional examples help in setting everything up.
+
+The Python backend is mainly added to allow for easier interfacing with many scientific libraries, such as numpy,
+scipy, tensorflow or pytorch.
+Creating static graphs with Matplotlib that can be rendered in the frontend could also be created.


Suggested change

# Telestion Python Backend

This repository contains everything to build Telestion backend services in Python.

Additional examples help in setting everything up.

The Python backend is mainly added to allow for easier interfacing with many scientific libraries, such as numpy,

scipy, tensorflow or pytorch.

Creating static graphs with Matplotlib that can be rendered in the frontend could also be created.

# Telestion Service Framework for Python

[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.10407142.svg)](https://doi.org/10.5281/zenodo.10407142)

![GitHub License](https://img.shields.io/github/license/wuespace/telestion)

This library provides a framework for building Telestion services in Python.

## Usage

```python

import asyncio

from telestion import start_service

async main():

service = await start_service(nats_disabled=True)

await service.publish('hello', lib.json_encode({'foo': 'bar'}))

await service.close()

if __name__ == '__main__':

asyncio.run(main())

```

TODO: Add short description of example code.

## Behavior Specification

The behavior of this library is specified in

the [Behavior Specification](https://docs.telestion.wuespace.de/Backend%20Development/service-behavior/).

This specification is also used to test the library.

The source code of the tests can be found in the repository under `/backend-features`.

## License

This project is licensed under the terms of the [MIT license](LICENSE).

(adapted from Deno backend)

fussel178 · 2025-01-08T18:41:09Z

    When I start the service with "--dev" without NATS
    Then the service should start
-    And the service should be configured with "NATS_URL" set to "localhost:4222"
+    And the service should be configured with "NATS_URL" set to either "localhost:4222" or "nats://localhost:4222"


That's a good point.
We need to test that every backend can handle both the full url and only the hostname+port part correctly.
But I think that's the scope of another pull request.

Can you please revert that change and implement proper handling of NATS_URL?

fussel178 · 2025-01-08T18:41:25Z

    And the service should connect to NATS
    And the service should be configured with "DATA_DIR" set to "/tmp"
-    And the service should be configured with "NATS_URL" set to "nats:4255"
+    And the service should be configured with "NATS_URL" set to either "nats:4255" or "nats://nats:4255"


Same as above. 😉

cb0s added 4 commits March 13, 2024 21:11

feat(backend-python): add basic telestion library for python

fa72884

feat(backend-python): add wrappers for nats client in lib.Service and…

944d2d0

… fix various bugs, add test script for development (will be deleted before merge into main)

feat(backend-python): improve api of lib for custom NatsClients by se…

6ec272b

…tting up the health check in designated function

fix(backend-python): fix support for custom options during startup an…

922acab

…d add basic doc strings

pklaschka requested changes Mar 22, 2024

View reviewed changes

cb0s added 2 commits December 21, 2024 14:54

Merge branch 'main' into feat/backend-python

383136e

feat: update implementation steps for e2e test

aa98860

fussel178 force-pushed the feat/backend-python branch from ccc0f69 to aa98860 Compare December 21, 2024 17:44

fussel178 and others added 3 commits December 21, 2024 18:52

test(gherkin): add assert_service_configured_with_alternative

77c9f96

fix(python): fix various bugs that were identified while implementing…

aad0a46

… test behaviour

cb0s requested review from fussel178 and pklaschka December 23, 2024 00:02

Update backend-python/README.md

50e3d24

Co-authored-by: Zuri Klaschka <pklaschka@users.noreply.github.com>

cb0s marked this pull request as ready for review December 23, 2024 00:06

fix(python): add support for '=' in additional args, add support for …

2db5a83

…custom arguments in `build_config()` and fix tests

fussel178 requested changes Jan 8, 2025

View reviewed changes

		return json.loads(msg, **loads_kwargs)


		def _prepare_nats_url(config: TelestionConfig) -> str:

	result = context.docker.containers.run(
	context.testbed,
	cmd,
	environment=env,
	auto_remove=True,
	network=nats_network_name(context),
	).splitlines()[-1].decode()

-# Telestion Python Backend
-This repository contains everything to build Telestion backend services in Python.
-Additional examples help in setting everything up.
-The Python backend is mainly added to allow for easier interfacing with many scientific libraries, such as numpy,
-scipy, tensorflow or pytorch.
-Creating static graphs with Matplotlib that can be rendered in the frontend could also be created.
+# Telestion Service Framework for Python
+[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.10407142.svg)](https://doi.org/10.5281/zenodo.10407142)
+![GitHub License](https://img.shields.io/github/license/wuespace/telestion)
+This library provides a framework for building Telestion services in Python.
+## Usage
+```python
+import asyncio
+from telestion import start_service
+async main():
+    service = await start_service(nats_disabled=True)
+    await service.publish('hello', lib.json_encode({'foo': 'bar'}))
+    await service.close()
+if __name__ == '__main__':
+    asyncio.run(main())
+```
+TODO: Add short description of example code.
+## Behavior Specification
+The behavior of this library is specified in
+the [Behavior Specification](https://docs.telestion.wuespace.de/Backend%20Development/service-behavior/).
+This specification is also used to test the library.
+The source code of the tests can be found in the repository under `/backend-features`.
+## License
+This project is licensed under the terms of the [MIT license](LICENSE).

Conversation

cb0s commented Mar 13, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Additional information

Related links

CLA

Uh oh!

pklaschka commented Mar 15, 2024

Uh oh!

cb0s commented Mar 16, 2024

Uh oh!

pklaschka commented Mar 17, 2024

Uh oh!

pklaschka commented Mar 17, 2024

Uh oh!

pklaschka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cb0s commented Dec 23, 2024

Uh oh!

fussel178 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cb0s May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cb0s commented Mar 13, 2024 •

edited by github-actions bot

Loading

fussel178 left a comment •

edited

Loading

cb0s May 9, 2025 •

edited

Loading