Remove hack in synth.py

[JustinBeckwith]

We currently have this hack in synth.py:

s.replace("src/v1/subscriber_client.js",
          "  }\n\s*/\*\*\n\s+\* The DNS address for this API service.",
          "\n    // note: editing generated code\n"
          "    this.waitForReady = function(deadline, callback) {\n"
          "      return subscriberStub.then(\n"
          "        stub => stub.waitForReady(deadline, callback),\n"
          "        callback\n"
          "      );\n"
          "    };\n"
          "    this.getSubscriberStub = function() {\n"
          "      return subscriberStub;\n"
          "    };\n"
          "\g<0>")

The code that's getting swapped in is creating lint issues in #641. It looks like there is an issue here in the gapic generator causing the pain:
googleapis/gapic-generator#2127

[callmehiphop]

For a super quick fix we could probably just update the regular expression.

Better fixes that would allow us to remove the hacks entirely might be:

• Have the gapic generator create comparable methods within clients
• Bypass gax entirely and manually create a Subscriber client for StreamingPull requests

[alexander-fenster]

@callmehiphop can you tell me more about the first option (have the gapic-generator create methods)? I'm happy to add any extra code to the codegen templates but I can't understand which code we need to make it not Pub/Sub-specific.

[callmehiphop]

@alexander-fenster absolutely! I think some sort of getter for the underlying gRPC client would do the trick.

To elaborate on why these two methods were stuffed in

waitForReady

Implementing retry logic for StreamingPull requests (bidi stream) isn't very straight forward (see googleapis/gax-nodejs#145), I consulted the gRPC team at one point and their suggestion was to make the requests and call waitForReady immediately after, this is the best indicator that our streams were opened successfully.

getSubscriberStub

My use case for this is very specific to PubSub. Each message delivered by the server is time sensitive and one of our goals is/should be to have as few response objects buffered as possible. Unfortunately whenever a stream is opened via gax, it pipes the request stream into a proxy stream which ends up doubling the size of our readable buffer, so 32 responses per stream by default. Since gax does not offer any kind of retry logic to bidi streams, I decided to bypass it entirely.

Alternatively adding streaming options like highWatermark to callOptions might be an ok solution as well.

[sduskis]

@JustinBeckwith, any objection to recategorize this as a "type: cleanup", since this doesn't affect users?

[feywind]

I tried removing the s.replace() hack back on Jan 27 but it looks like it's still broken. I haven't seen fixes for this on the horizon, so I'm going to go ahead and close it out for now.

[JustinBeckwith]

I'm happy to move this to another repo (gapic generator maybe?), but we do want to track these kinds of issues with the generators and synth hacks. cc @alexander-fenster

[feywind]

I'm fine with tracking it, but I've also been tasked with not leaving issues open if we're not fixing them in the particular repo soon. Should I create a new issue in gapic?

[alexander-fenster]

We will try to remove this hack when we move this library to TypeScript generator.

[JustinBeckwith]

In that case @alexander-fenster - you ok with just closing this? I figure we'll have a chance to discuss it in the PR that moves us to TS :)

[alexander-fenster]

Sure! (I actually thought it is closed :) )