Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize InfluxDB query & BugFix #26

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

benni336
Copy link
Contributor

According to https://www.influxdata.com/blog/writing-data-to-influxdb-with-python/
pushing data to influxdb is more efficient by merging data into one string.

@bohdan-s
Copy link
Owner

Hi,
I am not sure what this change achieves?
It is already collected into a single string before being pushed into InfluxDB?

@benni336
Copy link
Contributor Author

First I am not an influxdb expert. Just working with it the first time.
From my point of view sequence=[] is not a string but a list of strings. Looking at the debug output that gets very clear.

From influxdb docs I can quote and say your way is not wrong ( https://influxdb-client.readthedocs.io/en/stable/api.html#influxdb_client.WriteApi.write ). You can hand over a list. But than I read the linked article and think one should condense the data string before passing. For example: your list is much longer, as it always contains the name of the measurement variable. Just compare the debug output of our two codes.

If it is all about code simplicity, passing the original dict - just found out from the docs, that should be possible too - would be even "cleaner", than rewriting the values to strings.

Beside that, I would like to suggest to reduce the logging.debug() indentation. I don't think that a debug output is needed for every loop.

@bohdan-s
Copy link
Owner

bohdan-s commented Mar 7, 2022

Hi @benni336,
I think most of the commit here has already been addressed?
If you can re-base it I will approve :)
The git is now up to date with my current code, so should be easy to approve now.
Sorry

@bohdan-s
Copy link
Owner

bohdan-s commented Jul 7, 2022

Sorry, again merge conflicts on this commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants