Create pubsub subscriptions in the project specified in dataflow options

[rculbertson]

PubsubIO would create subscriptions in the same project as the topic. This fails for users who have permissions to subscribe to that topic, but do not have permissions to create subscriptions in that project. Instead create the subscription in the project specified in dataflow options. If no project is specified, then fallback to the topic project.

This is to address #220.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[rculbertson]

As requested, I signed the CLA.

[googlebot]

CLAs look good, thanks!

[dhalperi]

R: @mshields822 @dpmills

Mark, can you look at this w.r.t. your Beam work updating PubSubIO?

[mshields822]

LGTM
There are a few other places where we have assumes subscription project = topic project. I'll try to track those down.

[rculbertson]

@dhalperi Is this PR good to merge? It would be great to get this in the next release so we can remove the workarounds we currently have in place.

[dpmills]

LGTM

This looks fine to merge. One note, however: it won't fix streaming pipelines reading from Pubsub, since those don't use this code. This is just a default implementation that the batch and direct runners use. Fixing this for streaming pipelines will require changes to the Dataflow backend.

[davorbonaci]

@dpmills, this should currently be working as expected on the DataflowPipelineRunner, so no changes should be required. Am I missing something?

@rculbertson, would you be willing to add a unit test to make sure this scenario doesn't regress?

The change itself is fine. Two things worth noting:

• We should forward port this change to Apache Beam.
• This introduces a dependency of PubsubIO (core SDK) on DataflowPipelineOptions (Dataflow runner). We are separately trying to untangle these types of dependencies. We can figure out separately what's the right way of solving this.

We'll try to get this integrated early next week, and make sure it is part of the next release (1.6.0).

[rculbertson]

@davorbonaci Sure I can add a test sometime this week. Thanks!

[rculbertson]

@davorbonaci I've added a test to PubsubIOTest.java

[rculbertson]

@dhalperi @davorbonaci: Any update on getting this PR merged? Thanks!

[rculbertson]

Looks like this functionality was added in #278. Closing PR.

[dhalperi]

Ryan,

Thanks for the update. Is the new test you added still useful and relevant? If so, please rebase and reopen and I'll take a look!

Thanks!
Dan