Google Cloud: PubSub Topic and Subscription resources #3671

coffeepac · 2015-10-28T18:20:50Z

create the topics and subscriptions resources.

lwander · 2015-10-30T19:13:42Z

Looks good! Although, you're missing the push_config block shown here under the subscriptions resource.

Could you make the relevant updates to the documentation here and here?

Thanks for doing this!

sparkprime · 2015-11-02T22:52:43Z

All the other resources are prefixed by the name of the API that creates them, so instance becomes compute_instance and so on. Here, if you could use google_pubsub_topic and so on, that would be more consistent.

coffeepac · 2015-11-03T00:08:50Z

@lwander I haven't used the push_config guff and wasn't entirely sure how to test it. I'll take a look at it later tonight

@sparkprime I was tempted to pull the PR and redo so I'll make that change tonight as well.

coffeepac · 2015-11-03T05:09:42Z

@lwander the push config section is a bit beyond what I'm up to right now. if I have time at the end of my current project I'll add it but odds are someone else will need it first. documentation has been updated as well

@sparkprime resources/tests renamed to include pubsub hiearchy section

lwander · 2015-11-03T13:39:33Z

@coffeepac, in that case, are you OK if I pick up where you left off?

coffeepac · 2015-11-03T13:45:35Z

@lwander have at it my friend. Need me to do anything for that? First time passing off a PR on a project that isn't mine/internal.

lwander · 2015-11-03T15:22:00Z

@coffeepac, I think we need someone with commit privileges to turn this into a branch so you'll get credit for starting this off. Maybe @sparkprime could help?

sparkprime · 2015-11-03T18:46:22Z

This PR seems to also include some aws changes

coffeepac · 2015-11-03T19:33:16Z

yeah, looks like a bad/weird rebase. those shouldn't be there. I will sort that out tonight. my apologies.

coffeepac · 2015-11-04T04:31:53Z

@sparkprime @lwander okay! sorted out the funky rebase, I have no idea what I did but a little rebase back onto upstream/master and sanity returns. thanks git!

sparkprime · 2015-11-10T16:36:04Z

@lwander Shall I merge this and then you can do another PR to add whatever else, or are you planning on changing it in backwards-incompatible ways? In which case I won't do that :)

lwander · 2015-11-10T17:43:50Z

That sounds best

sparkprime · 2015-11-10T19:58:01Z

builtin/providers/google/resource_pubsub_subscription.go

+				ForceNew: true,
+			},
+
+			"topic_computed": &schema.Schema{


It doesn't look like this is documented and I don't see what use it can be. Can we remove it before merging this PR?

topic_computed is the canonical name of the topic that is required by the API for creating the subscription.

however, it is already being computed from the name schema value. I'll remove it shortly.

coffeepac · 2015-11-20T05:01:51Z

@sparkprime change made and build fixed. ready for launch.

sparkprime · 2015-11-23T22:16:52Z

Thanks, I'll merge when the current release freeze ends :)

coffeepac · 2015-11-23T22:17:43Z

Hooray! Thanks buddy!
On Nov 23, 2015 2:17 PM, "Dave Cunningham" [email protected] wrote:

Thanks, I'll merge when the current release freeze ends :)

—
Reply to this email directly or view it on GitHub
#3671 (comment).

coffeepac · 2015-12-02T05:31:20Z

@lwander @sparkprime my apologies for changing this after you had accepted it, but turns out I needed the ackDeadlineSeconds config param and while I was in there I added the pushConfig sections.

and a question: when is the code freeze until? thanks!

sparkprime · 2015-12-02T06:58:23Z

Sorry, it's actually been several days since the freeze ended but I had a bunch of other stuff going on as well as the Thanksgiving holidays and it slipped my mind.

Thanks for adding the extra stuff!

There's one general problem and that's that we tend to keep as close as possible to the Google APIs, which means that the push config stuff ought to be in a nested sub-block "push_config" with "push_endpoint" and "attributes" underneath it. The way this is done in Terraform right now is to use a list of 1 element to represent the block, and raise an error if the list is > 1 elements. There is an example here with the 'website' field: https://github.com/hashicorp/terraform/blob/master/builtin/providers/google/resource_storage_bucket.go

I can't merge this now as if I were to do that and we were then to change it, the change would not be backwards-compatible. However if you were to make that simple change to add the block, you would be the undisputed hero of Terraform Google pub/sub :)

coffeepac · 2015-12-02T07:03:22Z

Well, I do love being a hero...

On Tue, Dec 1, 2015 at 10:58 PM, Dave Cunningham [email protected]
wrote:

Sorry, it's actually been several days since the freeze ended but I had a
bunch of other stuff going on as well as the Thanksgiving holidays and it
slipped my mind.

Thanks for adding the extra stuff!

There's one general problem and that's that we tend to keep as close as
possible to the Google APIs, which means that the push config stuff ought
to be in a nested sub-block "push_config" with "push_endpoint" and
"attributes" underneath it. The way this is done in Terraform right now is
to use a list of 1 element to represent the block, and raise an error if
the list is > 1 elements. There is an example here with the 'website'
field:
https://github.com/hashicorp/terraform/blob/master/builtin/providers/google/resource_storage_bucket.go

I can't merge this now as if I were to do that and we were then to change
it, the change would not be backwards-compatible. However if you were to
make that simple change to add the block, you would be the undisputed hero
of Terraform Google pub/sub :)

—
Reply to this email directly or view it on GitHub
#3671 (comment).

coffeepac · 2015-12-02T07:25:21Z

@sparkprime I believe I have addressed your last two comments. thanks for your quick response once already!

sparkprime · 2015-12-02T07:44:56Z

Nice one! You're not going to believe this but there's another release freeze #4133

But I'll definitely merge it when that's over.

sparkprime · 2015-12-02T18:01:36Z

Freeze is over, but can I ask you to squash your commits. Last thing, I promise :)

…e that Conflicts: builtin/providers/google/provider.go builtin/providers/google/resource_subscription.go builtin/providers/google/resource_subscription_test.go golang pubsub SDK has been released. moved topics/subscriptions to use that Conflicts: builtin/providers/google/provider.go builtin/providers/google/resource_subscription.go builtin/providers/google/resource_subscription_test.go file renames and add documentation files remove typo'd merge and type file move add to index page as well only need to define that once remove topic_computed schema value I think this was used at one point but is no longer. away. cleanup typo adds a couple more config values - ackDeadlineSeconds: number of seconds to wait for an ack - pushAttributes: attributes of a push subscription - pushEndpoint: target for a push subscription rearrange to better match current conventions respond to all of the comments

coffeepac · 2015-12-02T18:55:26Z

@sparkprime looks like we're good to go with that squash. can you take a quick look and make sure i didn't miss anything?

Google Cloud: PubSub Topic and Subscription resources

lwander · 2015-12-02T20:40:09Z

🎊 🎊 🎊

Thank you @coffeepac!

coffeepac · 2015-12-02T20:46:25Z

Glad to help! Now to find the time to finish the bigquery PR 😃

ghost · 2020-04-29T02:24:32Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

radeksimko added enhancement provider/google-cloud labels Oct 28, 2015

apparentlymart changed the title ~~golang pubsub SDK has been released.~~ Google Cloud: PubSub Topic and Subscription resources Oct 28, 2015

coffeepac force-pushed the google-pubsub-clean branch from 0196d1f to e38d694 Compare November 4, 2015 04:30

sparkprime reviewed Nov 10, 2015
View reviewed changes

coffeepac mentioned this pull request Nov 20, 2015

Google Cloud: BigQuery DataSet and Table resources #3764

Closed

coffeepac force-pushed the google-pubsub-clean branch from ec2d9d9 to a88a69c Compare December 2, 2015 05:29

coffeepac force-pushed the google-pubsub-clean branch from 1c4cf3a to 6f7ef2f Compare December 2, 2015 18:42

sparkprime added a commit that referenced this pull request Dec 2, 2015

Merge pull request #3671 from 22Acacia/google-pubsub-clean

d6ae527

Google Cloud: PubSub Topic and Subscription resources

sparkprime merged commit d6ae527 into hashicorp:master Dec 2, 2015

ghost locked and limited conversation to collaborators Apr 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Google Cloud: PubSub Topic and Subscription resources #3671

Google Cloud: PubSub Topic and Subscription resources #3671

coffeepac commented Oct 28, 2015

lwander commented Oct 30, 2015

sparkprime commented Nov 2, 2015

coffeepac commented Nov 3, 2015

coffeepac commented Nov 3, 2015

lwander commented Nov 3, 2015

coffeepac commented Nov 3, 2015

lwander commented Nov 3, 2015

sparkprime commented Nov 3, 2015

coffeepac commented Nov 3, 2015

coffeepac commented Nov 4, 2015

sparkprime commented Nov 10, 2015

lwander commented Nov 10, 2015

sparkprime Nov 10, 2015

coffeepac Nov 10, 2015

coffeepac commented Nov 20, 2015

sparkprime commented Nov 23, 2015

coffeepac commented Nov 23, 2015

coffeepac commented Dec 2, 2015

sparkprime commented Dec 2, 2015

coffeepac commented Dec 2, 2015

coffeepac commented Dec 2, 2015

sparkprime commented Dec 2, 2015

sparkprime commented Dec 2, 2015

coffeepac commented Dec 2, 2015

lwander commented Dec 2, 2015

coffeepac commented Dec 2, 2015

ghost commented Apr 29, 2020

Google Cloud: PubSub Topic and Subscription resources #3671

Google Cloud: PubSub Topic and Subscription resources #3671

Conversation

coffeepac commented Oct 28, 2015

lwander commented Oct 30, 2015

sparkprime commented Nov 2, 2015

coffeepac commented Nov 3, 2015

coffeepac commented Nov 3, 2015

lwander commented Nov 3, 2015

coffeepac commented Nov 3, 2015

lwander commented Nov 3, 2015

sparkprime commented Nov 3, 2015

coffeepac commented Nov 3, 2015

coffeepac commented Nov 4, 2015

sparkprime commented Nov 10, 2015

lwander commented Nov 10, 2015

sparkprime Nov 10, 2015

Choose a reason for hiding this comment

coffeepac Nov 10, 2015

Choose a reason for hiding this comment

coffeepac commented Nov 20, 2015

sparkprime commented Nov 23, 2015

coffeepac commented Nov 23, 2015

coffeepac commented Dec 2, 2015

sparkprime commented Dec 2, 2015

coffeepac commented Dec 2, 2015

coffeepac commented Dec 2, 2015

sparkprime commented Dec 2, 2015

sparkprime commented Dec 2, 2015

coffeepac commented Dec 2, 2015

lwander commented Dec 2, 2015

coffeepac commented Dec 2, 2015

ghost commented Apr 29, 2020