Path template potential re-write

[jbolinger]

This is an example of a potential zero dependency replacement for the path template helpers to address the problems noted in #119 and #126 (excluding the inconsistency of symbolize_keys comment). It drops the need for rly and replaces the parsing logic with regular expressions.

@blowmage PTAL whenever you get a chance to verify if this change would actually be useful to you. I'm not too familiar with the tests it was impacting, or I might have tried to benchmark it myself, but if this helps we can find a way to get it in. Consider it a WIP for now.

DO NOT MERGE: the API is nearly identical but we may need to keep the original implementation around or include both (TBD).

[codecov-io]

Codecov Report

Merging #128 into master will decrease coverage by <.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
- Coverage   99.35%   99.35%   -0.01%     
==========================================
  Files          19       19              
  Lines        2029     2012      -17     
==========================================
- Hits         2016     1999      -17     
  Misses         13       13

Impacted Files	Coverage Δ
lib/google/gax/bundling.rb	99.37% <ø> (ø)	:arrow_up:
lib/google/gax/path_template.rb	100% <100%> (ø)	:arrow_up:
spec/google/gax/path_template_spec.rb	100% <100%> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9e8487d...90586ec. Read the comment docs.

[blowmage]

It looks like there is some change in behavior, but none of the current tests are catching it. I have some extra local test coverage from my previous spike at this and this is breaking that. How much do you care, or not care, about strict backwards compatibility?

[jbolinger]

Yeah, that doesn't surprise me. If you send me the tests I can try to update and see how far I can get it.

For backwards compatibility we've got options. I was thinking the simplest path might be to just include both implementations in the library for awhile and update any references in google-cloud-ruby to use the newer one to address the performance issues. I doubt this gets much use beyond that repo and given that this isn't a whole lot of code the duplication isn't too bad. Replacing would be cleaner but I just don't know if we've got the right coverage to do it with confidence.

[jbolinger]

On that same note, let me know how much of a difference this actually makes for you. I don't recall how significant the overhead was and it got labeled as a relatively low priority work item. If it's causing a significant amount of degradation across APIs I suspect our devrel folks would be more open to potentially disruptive changes.

[blowmage]

From our usage via the GAPIC layer, we won't see any impact from the behavior change. But if someone is using this class directly (not using the GAPIC layer), they may be impacted by the change.

I've been trying to get a performance benchmark all morning, it just hasn't happened yet.

[jbolinger]

I suspect usage from outside is very rare, but I don't really know or have a way to verify.

No worries on the timing. Get to it whenever you can. I was just looking for an excuse to write some Ruby for a change. Let me know if I can help!

[jbolinger]

Closing since the plan is to change this in the next major release.