TDengine Sink #931
TDengine Sink #931
Conversation
There was a problem hiding this comment.
Added some comments =)
Also note as a last piece, we typically add a doc in the docs/connectors/sinks folder, which outlines additional details on how the sink works, plus how to run tdengine locally for testing purposes (Let me know if you want what I did for that since I also used it for testing... happy to give you it!).
Feel free to copy another doc and make changes as needed.
| self, | ||
| host: str, | ||
| database: str, | ||
| measurement: MeasurementSetter, |
There was a problem hiding this comment.
If I recall correctly, this is one of the things we discussed that should probably be renamed, probably to supertable? I think you can largely keep it's behavior/functionality intact.
| host: str, | ||
| database: str, | ||
| measurement: MeasurementSetter, | ||
| table_name_key: str, |
There was a problem hiding this comment.
Related to measurement, this is something that probably makes sense to be named subtable. Also, it seems like it might make more sense to be Optional since it can be auto-generated with a hash.
Additionally, this seems like it should also accept a callable given the dynamic nature of them with the relationship with the tags, functioning similar to measurement.
For simplicity, you could even keep supertable to just accept a single string, and table_name_key would essentially become what measurement is now (and calling it subtable, of course =D)
There was a problem hiding this comment.
As an additional thought, wanted to clarify that we're open to you adjusting all this to what makes the most sense =) This was just what seemed most logical to me from my understanding of tdengine.
| :param table_name_key: A tag key whose value is used as the subtable name when writing to TDengine. | ||
| If the data does not contain this tag key, a hash value will be generated from the data as the subtable name. | ||
| :param fields_keys: an iterable (list) of strings used as InfluxDB line protocol "fields". | ||
| Also accepts a singl e-argument callable that receives the current message |
There was a problem hiding this comment.
singl e-argument
I think your keyboard likes to sneak spaces in =)
| ) | ||
| else: | ||
| raise ValueError("Either token or username and password must be provided") | ||
| if table_name_key: |
There was a problem hiding this comment.
Just a note to double-check this behavior if we change how this works.
| @@ -9,3 +9,4 @@ jsonschema>=4.3.0 | |||
| jsonlines>=4,<5 | |||
| rich>=13,<15 | |||
| jsonpath_ng>=1.7.0,<2 | |||
| python-dateutil | |||
There was a problem hiding this comment.
As a heads up, this dependency list is strictly for Quix Streams specific packages, so it should be moved from here =).
Connectors are managed in the pyproject.toml, added to [project.optional-dependencies] under both all and adding a new entry for the given connector, so tdengine=["python-dateutil"] (preferably with a version).
This enables doing pip install quixstreams[tdengine].
It'll also require you to add that same package to the conda/meta.yaml
There was a problem hiding this comment.
removed from here and added to conda/meta.yaml
| query_params["table_name_key"] = table_name_key | ||
| query_string = urlencode(query_params) | ||
| full_url = f"{base_url}?{query_string}" | ||
| self._client_args = { |
There was a problem hiding this comment.
Note that the request seems to be using port 80 by default, but tdengine seems to use port 6041 by default for the requests we're doing.
| cert_reqs = ssl.CERT_REQUIRED | ||
| else: | ||
| cert_reqs = ssl.CERT_NONE | ||
| self._client = urllib3.PoolManager( |
There was a problem hiding this comment.
I recommend that we try to confirm the DB exists here, and if not, try to create it. This is something we commonly do with db-based sinks during setup.
Plus, if the connection/credentials are incorrect, we'll also fail during setup, which allows us to add extra error handling in the Quix Platform =)
There was a problem hiding this comment.
Ultimately, it's helpful to not have users have to go and manually make assets where it can be helped (though obviously your UI can make that relatively trivial =D)
| from datetime import timezone as tz | ||
| from sys import version_info | ||
|
|
||
| from dateutil import parser |
There was a problem hiding this comment.
Check out how we handle connector-specific dependency imports (like in influxdb).
Basically here we want to encourage the pip install quixstreams[tdengine] approach if missing =)
Added the table naming template variable