Erb templating

[jkremser]

This is basically the same PR as the #49 but now targeting directly the master branch (not the 0.3.0 integration branch)

VCR_UPDATE=1 will now re-record the templates against the currently running Hawkular instance. By template I mean the recorded communication in yaml with <%= foobar => in it. This works for both VCR (done manually here https://git.io/vV5PM) and WebSocketVCR (it's a feature that is turned on by reverse_substitution: true)

VCR_OFF=1 is still working and can be used for running the tests against the live Hawkular instance, w/o recording it

Point of the whole PR is not to over write the VCR tapes over and over again, because the feed_id is randomly generated, or for those UUIDs in tests.

[coveralls]

Coverage decreased (-0.2%) to 94.352% when pulling f120d4b on Jiri-Kremser:erb-templating into 0edf190 on hawkular:master.

[pilhuhn]

LGTM - @abonas can you please have a look as well?

[abonas]

I will review this on Sunday

[abonas]

this section looks irrelevant for this PR?

[jkremser]

It's easier to keep the track of the changes this way, but if we want to be super correct, it hasn't been released yet, so it should not be in the CHANGES.rdoc.

If you are asking about the 'All the clients have been unified into one', that is a left over from previous work and isn't logically related to this PR.

It's gone.

[abonas]

I was trying to review this, but this PR is very big and also not separated to commits that might explain the changes separately. I left couple of comments, but I am definitely having difficulties understanding the code and the intention here. It might needs to be broken into smaller pieces. Or explain better in description what exactly is being done here.

[jkremser]

Sorry for that, the PR isn't actually so big, most of the code is the VCR cassette templates. We can have a call if you want and I can describe the changes.

The main intention here is not to have PRs like this in the future ;) It's quite annoying to overwrite the VCR cassettes in the git repo. On the other hand, having the stale data isn't good too. Heiko started to using the ERB templating in the VCR. This way, you can 'parametrize' some data (this feature https://relishapp.com/vcr/vcr/v/3-0-1/docs/cassettes/dynamic-erb-cassettes).

This partially works. Issue with the dynamic cassettes is that there is no easy to keep them up to date with the real implementation. That's why I raised this PR, it still uses the ERB dynamic cassettes when running with no special evn vars, but at the same time, if running as VCR_UPDATE=1 rspec, it updates all the templates. The way I do that is different for VCR and for WebSocketVCR.

For WebSocketVCR it's a built in feature in the gem, that can be turned on by reverse_substitution option. As for the normal VCR, I have to do it manually here: https://github.com/hawkular/hawkular-client-ruby/pull/56/files#diff-93830fa29d616f7c87903d08b5b1b29aR107

Side note1: in Hawkular the feed id (basically the ID of the agent that collect the data from WildFly server) is a random UUID, so if you run the rspec against a live server, it'll be different all the time. Hence, the need for some parametrization of the data.

[abonas]

why is "Removing the 'not yet released' changes in CHANGES.rdoc" a separate commit?

[abonas]

Sorry for that, the PR isn't actually so big, most of the code is the VCR cassette templates. We can have a call if you want and I can describe the changes.

The main intention here is not to have PRs like this in the future ;) It's quite annoying to overwrite the VCR cassettes in the git repo. On the other hand, having the stale data isn't good too. Heiko started to using the ERB templating in the VCR. This way, you can 'parametrize' some data (this feature https://relishapp.com/vcr/vcr/v/3-0-1/docs/cassettes/dynamic-erb-cassettes).

This partially works. Issue with the dynamic cassettes is that there is no easy to keep them up to date with the real implementation. That's why I raised this PR, it still uses the ERB dynamic cassettes when running with no special evn vars, but at the same time, if running as VCR_UPDATE=1 rspec, it updates all the templates. The way I do that is different for VCR and for WebSocketVCR.

For WebSocketVCR it's a built in feature in the gem, that can be turned on by reverse_substitution option. As for the normal VCR, I have to do it manually here: https://github.com/hawkular/hawkular-client-ruby/pull/56/files#diff-93830fa29d616f7c87903d08b5b1b29aR107

reverse_substitution - is the name coming from somewhere built in? it is not very clear/how it relates to templates.

Side note1: in Hawkular the feed id (basically the ID of the agent that collect the data from WildFly server) is a random UUID, so if you run the rspec against a live server, it'll be different all the time. Hence, the need for some parametrization of the data.

yes, the feature itself and its benefits are quite clear :)

[coveralls]

Coverage decreased (-0.2%) to 94.352% when pulling dbe612b on Jiri-Kremser:erb-templating into 0edf190 on hawkular:master.

[jkremser]

reverse_substitution - is the name coming from somewhere built in? it is not very clear/how it relates to templates.

It's an option for the WebSocketVCR, if it's used, it basically records/creates the templates. I.e. such cassettes that have variables on places where the values should be. Normal substitution goes the other way around, hence the name reverse substitution.

https://git.io/vw9RY
https://git.io/vw9RQ

[jkremser]

why is "Removing the 'not yet released' changes in CHANGES.rdoc" a separate commit?

Right, it doesn't make sense to introduce the change and then remove it withing the 1 PR, agreed. Can we just use the squash&merge thing on GitHub at the end? It's less work. (https://github.com/blog/2141-squash-your-commits)

[abonas]

Right, it doesn't make sense to introduce the change and then remove it withing the 1 PR, agreed. Can we just use the squash&merge thing on GitHub at the end? It's less work. (https://github.com/blog/2141-squash-your-commits)

I never used it this way, I'd typically squash or git amend manually. does it make sense to do it for a change that shouldn't have entered in the first place?

[coveralls]

Coverage decreased (-0.2%) to 94.352% when pulling 6788789 on Jiri-Kremser:erb-templating into 0edf190 on hawkular:master.

[jkremser]

does it make sense to do it for a change that shouldn't have entered in the first place?

Well, if you squash together

+foo

and

-foo

there will be no foo in the resulting commit. But I've already done that, because I did also the s/e/example/

[coveralls]

Coverage decreased (-0.2%) to 94.352% when pulling 6788789 on Jiri-Kremser:erb-templating into 0edf190 on hawkular:master.

[abonas]

ok looking good :)

[jkremser]

@pilhuhn could you please merge? If you rebase your pr on this, it'll fix the travis failures