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

clarify mysql target documentation #335

Merged
merged 7 commits into from
Sep 19, 2018

Conversation

rgitzel
Copy link
Contributor

@rgitzel rgitzel commented Sep 17, 2018

@rgitzel rgitzel force-pushed the issue-334-clarify-mysql-target branch from 2291e17 to e67855e Compare September 17, 2018 20:40
@rgitzel rgitzel changed the title Issue 334 clarify mysql target clarify mysql target documentation Sep 17, 2018
@rgitzel rgitzel changed the title clarify mysql target documentation clarify mysql target documentation - DON'T MERGE, HAS QUESTIONS Sep 17, 2018
- e.g. 'topic', 'payload' and '_dtiso' as above
- note that these all must be VARCHAR columns; timestamp columns are [not yet supported](https://github.com/jpmens/mqttwarn/issues/334#issuecomment-422141808)
- the 'fallback' column, as noted below
- #TODO: how is this different from 'payload'?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fallback column gets all the JSON elements which have not been otherwise stored. e.g. if payload contains { name: Jane, number: 42 } (think JSON please), and we have a column name in the DB, then fallback will get {number: 42} whereas payload would have the full original payload.

Copy link
Contributor Author

@rgitzel rgitzel Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried it, and it seems that 'payload' and 'full' are the same. I added the 'full' column and posted a new message:

+------+------------+-----------------------------------------------------+-----------------------------------------------------+-----------------------------+-------+
| id   | name       | payload                                             | full                                                | _dtiso                      | topic |
+------+------------+-----------------------------------------------------+-----------------------------------------------------+-----------------------------+-------+
|   90 | Jane Jolie | { "name" : "Jane Jolie", "id" : 90, "number" : 17 } | NULL                                                | 2018-09-17T20:20:31.889002Z | db    |
|   91 | Jane Jolie | { "name" : "Jane Jolie", "id" : 91, "number" : 17 } | { "name" : "Jane Jolie", "id" : 91, "number" : 17 } | 2018-09-18T20:44:51.662836Z | db    |

Looking at the code, I see where the fallback column is set: https://github.com/jpmens/mqttwarn/blob/master/services/mysql.py#L67

But there's no point where fields are removed. We only know which fields should be removed after add_row is called, which is too late.

README.md Outdated

First it requires the [MySQLDb](http://mysql-python.sourceforge.net/) library to be installed.
- _Debian/Ubuntu_: `sudo apt-get install -y python-mysqldb`
- TODO: do we have any others?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpmens ?

This isn't the easiest library to install. :(

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not my fault ... :) I suggest leavig it, and/or saying "install with pip" in which case dependencies are automatically pulled in. Probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a way to do it with pip so changed the instructions.

Copy link
Collaborator

@jpmens jpmens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, @rgitzel, as usual. Thank you.

README.md Outdated

First it requires the [MySQLDb](http://mysql-python.sourceforge.net/) library to be installed.
- _Debian/Ubuntu_: `sudo apt-get install -y python-mysqldb`
- TODO: do we have any others?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not my fault ... :) I suggest leavig it, and/or saying "install with pip" in which case dependencies are automatically pulled in. Probably.

README.md Outdated
| 90 | Jane Jolie | { "name" : "Jane Jolie", "id" : 90, "number" : 17 } | 2018-09-17T20:20:31.889002Z | my/2 |
+------+------------+-----------------------------------------------------+-----------------------------+-------+
```
Here, the plugin pulled values for the new columns from standard mqttwarn meta-data.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you please s/meta-data/metadata/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@jpmens jpmens merged commit c3f7a4f into mqtt-tools:master Sep 19, 2018
@jpmens
Copy link
Collaborator

jpmens commented Sep 19, 2018

Thank you very much. Merged.

@jpmens jpmens changed the title clarify mysql target documentation - DON'T MERGE, HAS QUESTIONS clarify mysql target documentation Sep 19, 2018
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.

None yet

2 participants