Additional improvements to persistence article #535

bgilmer77 · 2017-10-20T22:42:40Z

Addressing comments left over from #527

Changed that -> than
Resolved clashing headlines
Strategies section: added default and explained its operation
Changed persistence extensions from code block to itemized list

Addressing comments left over from openhab#527 - Changed that -> than - Resolved clashing headlines - Strategies section: added `default` and explained its operation - Changed persistence extensions from code block to itemized list

bgilmer77 · 2017-10-20T22:44:09Z

So... @ThomDietrich, here is a new PR.

First commit addresses all but two of your comments on #527.

Two items are left:

Example: I would vote to remove the ALL example...
I assume you are referring to:

: strategy = everyChange, everyDay, restoreOnStartup

If so, we could change this to something like:

node14_humidity: strategy = everyChange, restoreOnStartup

or, do you have a better example? Am I understanding your comment correctly?

Your second unaddressed comment: Add a section to justify cron settings (when can what be recommended)
I am not sure what you mean here. Do you mean to create an example that, for example persists temperature every five minutes for graphing later? For example:

Strategies {
everyFiveMinutes : "0 0/5 * * * ?" <- IS THIS SYNTAX CORRECT??
}

Items {
node14_temperature : strategy = everyFiveMinutes
}

And add text in front of this example pointing readers to the cron documentation for syntax and explaining the example?

Got a better suggestion?

BTW, I have some other changes I will submit on

Confectrician

Hi Brad,

Some small comments from my side. :)

Cheers
Jerome

Confectrician · 2017-10-22T11:28:48Z

configuration/persistence.md

  ...
 }
 ```

+If no strategy is specified in an `itemlist` as described in the Items section below, the `default` strategy will be applied. The `default` parameter is optional and may be omitted.


Please break down to one line per sentence.

This may be a bit confusing for beginners.
On one side the default parameter is used when no other is given but in the next sentence this is explained as optional.
Maybe it would be good to make clear that default has to be set when one doesn't want to use explicit strategies only.

Its maybe a really small thing, but i stocked while reading through it and wanted to mention it.

I agree completely. How about this...

The default parameter assigns a strategy to be used if one is not specified in the Items section below.
The default parameter may be omitted from the Strategies section, but if and only if a strategy is provided in each line of the Items section.

Replace the text in the Items section with the following:
Note: if the strategy portion of the itemlist is omitted, the default strategy specified in the Strategies section will be applied.

Sounds good. 👍

Confectrician · 2017-10-22T11:29:51Z

configuration/persistence.md

    <itemlist2> [-> "<alias2>"] : [strategy = <strategyX>, <strategyY>, ...]
    ...

 }
 ```

+Note that you may omit the `strategy` portion of the `itemlist` so long as you have provided a `default` strategy in the `Strategies` section.


so long as => as long as

Yeah - 'so long as' and 'as long as' are superfine distinctions in English. 'as long as' is much more common, so this is what I should use in a document being read by people from all over the world. In any case, I am proposing to replace all of this anyway, but thank you for pointing this out.

Ahh - from dictionary.cambridge.org (what do they know about English??!!)
As long as: refer to the intended duration of a plan or idea
As long as and so long as: means 'provided that", 'providing that" or 'on condition that'. So long as is a little informal.

To be honest i didn't think of any rules. It sounded strange to me while reading. 😄
That was my intention to comment it here.

I agree. Well, I will rewrite the section and drop this strange wording anyway.

@Confectrician

- Reworded some of tne introductory material - Reorganized and changed some sub-section titles in Strategies and Items area to improve comprehension. - Item Persistence Configuration -> Persistence Configuration - Added Persistence Trigers section - Strategies section -> Strategies - Items section -> Items - Addressed comments by @Confectrician

bgilmer77 · 2017-10-23T14:18:46Z

@ThomDietrich please see comment above about changing the example from "persist absolutely everything all the time" to a specific example of an Item being persisted. Is this okay, or would you like to suggest a different one?

FWIW, I stopped at the "A valid persistence configuration file might look like this:"

ThomDietrich · 2017-10-23T14:25:50Z

configuration/persistence.md


 A complete list of supported persistence add-ons may be found in the [persistence]({{base}}/addons/persistence.html) section of the on-line openHAB documentation.

-## Persistence Add-on Configuration
+## Add-on Configuration


Once about the "Configuration" headline issue. I think it would best be solved if you simply removed this one.
wdyt?

Yes - that makes sense.

ThomDietrich · 2017-10-23T14:28:15Z

configuration/persistence.md


-Each persistence add-on you install will need to be configured.
 Please refer to the specific [on-line documentation]({{base}}/addons/persistence.html) for your selected persistence add-on for configuration instructions.


In most other articles we are linking the subject directly. I.e.

Please refer to the documentation of available persistence service add-ons for configuration instructions.

(besides I for one would write online instead of on-line...?)

Changed to, "Please refer to the available persistence service add-on documentation for your selected persistence add-on for configuration instructions."

ThomDietrich · 2017-10-23T14:30:59Z

configuration/persistence.md


-Persistence Strategies are configured in a file named `<persistenceservice>.persist`, stored in `$OPENHAB_CONF/persistence` ("persistenceservice" is replaced by the name of your add-on (e.g. `rrd4j.persist`)).
+Persistence Strategies are configured in a file named `<persistenceservice>.persist`, stored in `$OPENHAB_CONF/persistence` (Replace "persistenceservice" with the name of your persistence add-on (e.g. `rrd4j.persist`)).


double brackets? 😕

How about one sentence to introduce the concept. Short example: "Configuration defines which Item, when, where."

ThomDietrich · 2017-10-23T14:36:03Z

configuration/persistence.md


-This section allows you to define strategies and to declare a set of default strategies to use for this persistence service.  The syntax is as follows:
+This section allows you to define one or more `Strategies` that will be applied to `Items` (you specifiy the `Items` in the `Items` section).


The Strategies section allows to define one or more persistence strategies. A strategy defines the event that leads to the storage of an Item's state in the persistence service.

Two points.

I tend to write technical information in a more personal style than some people. If this is not your preference, then I can go to an impersonal style, but I find it reads in a very strangely sometimes. Also, a little humanity can go a long way if it does not interfere or get too chatty. But this is your project and your call. Let me know what you prefer.

"allows to define" is a broken English sentence. You need an object. So the alternatives are, "The Strategies section allows YOU to define..." or the more impersonal, "The Strategies section allows ONE to define...". We could restructure it as follows and dodge the point... "The Strategies section defines one or more persistence strategies."

Thoughts?

Thought 1: This is not my project! 😄 I'm merely the guy who joined the game a few months earlier than you 😉 You have an argument for the "you" and with that I'm fine to leave it in.

Regarding 2: Interesting. I am pretty sure that I'm using all three versions depending on my mood ^^ I'll try to remember that.

Cool - we will leave "you" in. And okay on not being your project - got it. But don't want to step on your work either.

Regarding 2, the first is definitely broken, so it would be best to avoid this construction.

While we are on this topic, we should really avoid the English passive voice. The example in Wikipedia is great...

Good - Brutus stabbed Caesar
Bad - Caesar was stabbed by Brutus

The subject of the sentence should be the "doer" - the person/thing doing the doing.

Good - "The Binding updates the Item"
Bad - "The Item is updated by the binding"

ThomDietrich · 2017-10-23T14:40:17Z

configuration/persistence.md


-This section allows you to define strategies and to declare a set of default strategies to use for this persistence service.  The syntax is as follows:
+This section allows you to define one or more `Strategies` that will be applied to `Items` (you specifiy the `Items` in the `Items` section).
+This section also defines a default strategy.


Combine with the explanation below (either here or there).

ThomDietrich · 2017-10-23T14:42:48Z

configuration/persistence.md


 - everyChange: persist the Item state whenever its state has changed
 - everyUpdate: persist the Item state whenever its state has been updated, even if it did not change
- restoreOnStartup: load and initialize the last persisted Item state on openHAB startup (if the Item state is undefined (`UNDEF`)).  See below.
+- restoreOnStartup: load and initialize the last persisted state of the Item on openHAB startup (if the Item state is undefined (`UNDEF`)).  See below.


"restoreOnStartup will load and ..." ?

Well, the way it is currently written (this is the original language from the Wiki) it matches the other points. RE:

everyChange: persist the...
everyUpdate: persist the...
restoreOnStartup: load and initialize the...

I could change as follows:

restoreOnStatup: will load and initialize the...

but then I think the others should follow the same form:

everyChange: will persist the...
everyUpdate: will persist the...

My two cents are that the "will" is extra, but I am happy to put it in if you like. But I do feel they should all be the same.

Sorry, this is not the first time I'm misleading you with super short comments.

Yes all three

Please add code fences

I'm fine with either of

restoreOnStartup: load...

restoreOnStartup - load...

restoreOnStartup will load...

Up to you!

OK. Not misleading. But...

Two of them cause a persistence action. The last one causes a restore action. So 1 & 2 should be "persist" or "store". Number 3 should be "load". Agree?

ThomDietrich · 2017-10-23T14:43:21Z

configuration/persistence.md

-### Items section
+#### Cron Persistence Triggers
+openHAB uses [Quartz](http://www.quartz-scheduler.org/documentation/quartz-2.1.x/quick-start.html) for time-related cron events.
+In the `Strategies` syntax example above, "cronexpression" is a Quartz cron-like expression.


cronexpression

ThomDietrich · 2017-10-23T14:43:35Z

configuration/persistence.md


 ```java
 Strategies {
  <strategyName1> : "cronexpression1>"
+  <strategyName2> : "cronexpression2"


With our without <>?

Right - ERROR. Will change to : "cronexpression1"

ThomDietrich · 2017-10-23T14:44:36Z

configuration/persistence.md

@@ -76,6 +90,8 @@ where `<itemlist>` is a comma-separated list consisting of one or more of the fo
 - `<itemName>` a single Item identified by its name. This Item can be a group Item.  But note that only the group value will be persisted.  The value of the individual group members will not be persisted using this option.
 - `<groupName>*` - all members of this group will be persisted, but not the group itself. If no strategies are provided, the default strategies that are declared in the first section are applied.  Optionally, an alias may be provided if the persistence service requires special names (e.g. a table to be used in a database, a feed id for an IoT service, etc.)

+Note: if the `strategy` portion of the `itemlist` is omitted, the `default` strategy specified in the `Strategies` section will be applied.


Please add an example below.

How about the following:

For example, given an Item named GF_Hall_Light, if default in the Strategies section is set to everyChange, then the following Items section,

Items { GF_Hall_Light }

will cause the state of GF_Hall_Light to be persisted on every change.

ThomDietrich · 2017-10-23T14:45:44Z

configuration/persistence.md

@@ -139,20 +155,40 @@ You can easily imagine that you can implement very powerful rules using this fea



In the unchanged part above, please capitalize headlines

Okay - will ensure all headlines are capitalized in the entire article.

This comment has been addressed

ThomDietrich · 2017-10-23T14:54:26Z

I would vote to remove the ALL example...

Yes your example would be okay with me. However to show good use of the restoreOnStartup feature, let's go with an item that doesn't come from a binding? First idea by myself:

Heating_Mode, Notifications_Active: strategy = everyChange, restoreOnStartup

Add a section to justify cron settings (when can what be recommended)

No I think the cron topic is addressed quite well.

I was aiming at the issue discussed in various places: Which persistence is best used for which kinds/types of Items (slow/fast changing, discrete/continuous, ...) However I realize that this is not easy to answer. Maybe topic for a later PR.

ThomDietrich · 2017-10-23T14:56:31Z

configuration/persistence.md

+#### Cron Persistence Triggers
+openHAB uses [Quartz](http://www.quartz-scheduler.org/documentation/quartz-2.1.x/quick-start.html) for time-related cron events.
+In the `Strategies` syntax example above, "cronexpression" is a Quartz cron-like expression.
+See the  [Quartz cron tutorial](http://www.quartz-scheduler.org/documentation/quartz-2.1.x/tutorials/crontrigger) for examples of valid time-related trigger expressions.


The cron topic is also addressed under rules. There the description might also not be perfect but at least some useful links are given. Imho we should link to there instead of repeating incomplete info here!? http://docs.openhab.org/configuration/rules-dsl.html#time-based-triggers

Yes - I looked at the Rules page when I wrote this. I agree that, to the greatest extent possible, there should be "one source of the truth" rather than scattering information around in different places. So...

Does this mean that there should be a separate article on Quartz and that both the Rules and Persistence articles should link to it?

Do you know for sure if you can use "midnight" and "noon" in persistence rules? This is why I did not point to the Items article. (Also, I did some brief digging but did not find any definite information about whether terms other than those two are supported. Or...

I can take the quick way out and just point to the Rules article (but there is no sub-link that I can point to that would take them straight to the Time section of the Rules document. (I could add that as a quick PR.)

Thoughts?

Okay - I will change the examples to your heating example. I agree something that is not updated by a binding is better.

I think it would be okay to link to the rules article. I'm not sure what you mean by sub-link. This one should be it?
http://docs.openhab.org/configuration/rules-dsl.html#time-based-triggers

Regarding midnight and all other doubts: Honestly you can imho ignore that for now. There is a totally new scheduler at the verge of being merged to eclipse smarthome. We'll have to touch documentation because of that soon enough.

Okay - I will simply link to that article.

Yep - that is what I meant by the sub-link. (But I did not see the little chain link symbol on the documentation page in HTML, so I assumed the links had not been added. I need to go look at the source for the Rules page.)

Okay - I was just concerned that someone might dive into the Rules link, see the "midnight" terminology and immediately put it into a persistence definition only to have it blow up (or work - I have no idea whether it is supported in persistence configs or not). Not a big deal.

- Deleted one configuration headline - Added intro sentence to configuration section - Combined all text about default in one section of the document - corrected cronexpression angle bracket error - added code fence around everyChange, etc. - Removed detail of Quartz stuff and just added a pointer to Rules article - Replaced "persist everything" example to persisting Heating_Mode, etc..." - Added example of how default works.

bgilmer77 · 2017-10-23T18:01:15Z

Question - this PR is getting pretty complicated. Do we want to merge what we have (after any additional comments) and then I will work on the remaining sections? Or do you prefer to have all of the updates for this pass in one PR?

ThomDietrich · 2017-10-23T18:16:03Z

Hmm it's not that big yet, let's stay with one PR.

Will you continue with the second half of the article? I guess there are some more things we can improve.

bgilmer77 · 2017-10-23T19:52:26Z

Okay - please don't merge until tomorrow. I saw a few things I want to tweak before we close this. I think there are other issues, but they are larger than just this. For example, putting the cron stuff all in one place and pointing both this and Rules to it. I also think we need an article (tutorial?) on databases, as you mentioned. It would be a Good Thing to be able to point people to that.

So I will work more on this in the next 24 hours or so.

bgilmer77 · 2017-10-23T20:00:16Z

Ahh - your comment about doing a merge soon had to do with the sitemaps article - I was reading the email notification and did not click through to the GitHub link until just now. Anyway, more input coming on this article.

- Numerous small changes to improve readability

bgilmer77 · 2017-10-31T07:56:51Z

@ThomDietrich I have now completed my nit pass through the entire Persistence article. There is one TODO open under Workaround 1 for the issue of the rules engine and persistence restoration happening at the same time. The note says we need an example of the workaround. I don't feel comfortable writing this even though it is simple, so I just left the TODO open.

Please look this over and see if it is ready to merge.

- Existing text was not clear about the syntax one would use with a Persistence Extension in order to specify a persistence store in cases where multiple stores are available. - Clarified language and added an example.

bgilmer77 · 2017-10-31T13:45:27Z

One thing- I would like to fix the code-fenced text around the persistence extensions, but I do not know how to "escape" an angle bracket so that it remains visible. In the text now, [item] (but with angle brackets instead of square brackets) does not appear to render normally. What is the secret?

ThomDietrich · 2017-11-02T13:43:49Z

Hey Brad,

Angle brackets can even in Markdown be used to include HTML code tags. Your <Item> is interpreted as an (invalid) HTML tag. If you need the actual characters, you need to HTML-escape them with < and >.
In the specific case I'd however suggest to put the whole strings in code blocks like <item>.historicState(AbstractInstant).
Also I'm not sure why there are two empty lines between all of them!? May we just remove those or what's your intention there? Remeber to check out http://www.cirosantilli.com/markdown-style-guide/#lists

- Removed extra lines - Added code fences Signed-off-by: Brad Gilmer <[email protected]> (github: bgilmer77)

bgilmer77 · 2017-11-03T15:24:28Z

@ThomDietrich I think all of your comments have been addressed (hopefully). I have no idea why there were all the extra spaces - they have been removed. I have also added code fences, and everything looks just as it should, in my opinion, but let me know what you think.
Standby - I just looked at the Persistence Extensions and still think this formatting stinks. I am going to try a table instead...

- Changed Persistence Extensions section from a non-enumerated list to a table - Fixed a spelling error in the section on restoring items after a restart Signed-off-by: Brad Gilmer <[email protected]> (github: bgilmer77)

bgilmer77 · 2017-11-03T15:57:04Z

Reformatted the Persistence Extensions as a table. I like this much better. Okay with you?

BTW, I installed jekyll and can now render pages locally - what a Fantastic Thing!!!!

Signed-off-by: Thomas Dietrich <[email protected]> (github: ThomDietrich)

ThomDietrich · 2017-11-03T17:14:10Z

The table is also a good option! I've added a proper header to it with my commit. I believe the article can still be improved but it's a great start and I'd say let's continue and merge. New PRs can be created for further aspects.

A local Jekyll installation is great! Did you also check out the Vagrant machine?
One more thing I wonder is if you are using the remark linting functionality I've introduced recently in #418 and #423 ? It helps to check the documents you edit for markdown errors. You'll need nodejs installed and the Atom editor for in-line error highlighting. See this file for more details: https://github.com/openhab/openhab-docs/blob/gh-pages/package.json - If you need further help please open an issue.

Confectrician · 2017-11-03T17:20:09Z

A local Jekyll installation is great! Did you also check out the Vagrant machine?

To be honest:
I have switched from Vagrant to a local jekyll installation, since this didnt work well on windows.
Especially refreshing a file failed and didnt regenerate the site.
With the linux subsystem for windows installing jekyll is really easy.

We should maybe add some info about this in the docs too, for interested users and give them a starting point.

ThomDietrich · 2017-11-03T17:23:49Z

Yeah the refreshing is an issue I've also struggled with for quite some time...

Yes please :) https://github.com/openhab/openhab-docs/blob/gh-pages/README.md

Additional improvements to persistence article

02f73a8

Addressing comments left over from openhab#527 - Changed that -> than - Resolved clashing headlines - Strategies section: added `default` and explained its operation - Changed persistence extensions from code block to itemized list

Confectrician reviewed Oct 22, 2017

View reviewed changes

ThomDietrich reviewed Oct 23, 2017

View reviewed changes

Finishes "nit" pass through Persistence article

4d181fc

- Numerous small changes to improve readability

Expand explanation of how one may specify persistence for an Item

a6768d6

- Existing text was not clear about the syntax one would use with a Persistence Extension in order to specify a persistence store in cases where multiple stores are available. - Clarified language and added an example.

Correct formating of Persistence Extensions

52ed5d4

- Removed extra lines - Added code fences Signed-off-by: Brad Gilmer <[email protected]> (github: bgilmer77)

Format Persistence Extensions as a table

7f5c0df

- Changed Persistence Extensions section from a non-enumerated list to a table - Fixed a spelling error in the section on restoring items after a restart Signed-off-by: Brad Gilmer <[email protected]> (github: bgilmer77)

Fix markdown table style

c1cd6c6

Signed-off-by: Thomas Dietrich <[email protected]> (github: ThomDietrich)

ThomDietrich merged commit cd91117 into openhab:gh-pages Nov 3, 2017

bgilmer77 deleted the persistence-additional-edits branch November 3, 2017 18:39

Confectrician added this to the 2.x.x milestone Nov 18, 2022


		Each persistence add-on you install will need to be configured.
		Please refer to the specific [on-line documentation]({{base}}/addons/persistence.html) for your selected persistence add-on for configuration instructions.


		Persistence Strategies are configured in a file named `<persistenceservice>.persist`, stored in `$OPENHAB_CONF/persistence` ("persistenceservice" is replaced by the name of your add-on (e.g. `rrd4j.persist`)).
		Persistence Strategies are configured in a file named `<persistenceservice>.persist`, stored in `$OPENHAB_CONF/persistence` (Replace "persistenceservice" with the name of your persistence add-on (e.g. `rrd4j.persist`)).


		This section allows you to define strategies and to declare a set of default strategies to use for this persistence service. The syntax is as follows:
		This section allows you to define one or more `Strategies` that will be applied to `Items` (you specifiy the `Items` in the `Items` section).

		@@ -139,20 +155,40 @@ You can easily imagine that you can implement very powerful rules using this fea

Additional improvements to persistence article #535

Additional improvements to persistence article #535

Conversation

bgilmer77 commented Oct 20, 2017

bgilmer77 commented Oct 20, 2017

Confectrician left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgilmer77 commented Oct 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomDietrich Oct 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomDietrich commented Oct 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgilmer77 commented Oct 23, 2017

ThomDietrich commented Oct 23, 2017

bgilmer77 commented Oct 23, 2017

bgilmer77 commented Oct 23, 2017

bgilmer77 commented Oct 31, 2017

bgilmer77 commented Oct 31, 2017

ThomDietrich commented Nov 2, 2017 • edited Loading

bgilmer77 commented Nov 3, 2017 • edited Loading

bgilmer77 commented Nov 3, 2017

ThomDietrich commented Nov 3, 2017 • edited Loading

Confectrician commented Nov 3, 2017

ThomDietrich commented Nov 3, 2017

ThomDietrich Oct 23, 2017 •

edited

Loading

ThomDietrich commented Nov 2, 2017 •

edited

Loading

bgilmer77 commented Nov 3, 2017 •

edited

Loading

ThomDietrich commented Nov 3, 2017 •

edited

Loading