Add support for message_storage_policy to customize locations #15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paulpalamarchuk
force-pushed
the
add_message_storage_policy
branch
2 times, most recently
from
1d50929
to
624acb2
Compare
paulpalamarchuk
changed the title
paulpalamarchuk
force-pushed
the
add_message_storage_policy
branch
5 times, most recently
from
aac733d
to
dfbb060
Compare
paulpalamarchuk
requested review from
nick4fake
and removed request for
nick4fake
nick4fake
previously approved these changes
Oct 28, 2019
paulpalamarchuk
force-pushed
the
add_message_storage_policy
branch
from
dfbb060
to
2dd66fc
Compare
paulpalamarchuk
force-pushed
the
add_message_storage_policy
branch
from
2dd66fc
to
6089d4e
Compare
aaron-lane
previously requested changes
Oct 28, 2019
There was a problem hiding this comment.
Thanks @paulpalamarchuk!
Can you please provide evidence that this change solves the permanent diff?
paulpalamarchuk
force-pushed
the
add_message_storage_policy
branch
from
9334cfd
to
baa66c2
Compare
morgante
suggested changes
Oct 30, 2019
README.md
Outdated
| @@ -73,6 +74,7 @@ In order to execute this module you must have a Service Account with the followi | |||
| #### Roles | |||
|
|
|||
| - `roles/pubsub.editor` | |||
| - `roles/dataproc.viewer` or simular, that allows `compute.regions.list` | |||
There was a problem hiding this comment.
Suggested change
| - `roles/dataproc.viewer` or simular, that allows `compute.regions.list` | |
| - `roles/dataproc.viewer` or similar, that allows `compute.regions.list` |
main.tf
Outdated
| allowed_persistence_regions = var.allowed_persistence_regions == null ? data.google_compute_regions.available.names : var.allowed_persistence_regions | ||
| } | ||
|
|
||
| data "google_compute_regions" "available" { |
There was a problem hiding this comment.
I'd really like to avoid adding this requirement if we don't absolutely need to.
Instead of doing this, I'd like to update this PR so the message_storage_policy block is only configured if regions are provided.
| @@ -22,6 +22,13 @@ resource "google_pubsub_topic" "topic" { | |||
| project = var.project_id | |||
| name = var.topic | |||
| labels = var.topic_labels | |||
|
|
|||
| dynamic "message_storage_policy" { | |||
| for_each = var.message_storage_policy | |||
There was a problem hiding this comment.
for_each should be of a list, not a map. The way this is structured is very weird.
There was a problem hiding this comment.
I decided to use such structure, because for_each accepts a map or a set
There was a problem hiding this comment.
I guess it works, but it's non-intuitive.
paulpalamarchuk
force-pushed
the
add_message_storage_policy
branch
from
baa66c2
to
6826570
Compare
paulpalamarchuk
force-pushed
the
add_message_storage_policy
branch
from
6826570
to
9fd7b46
Compare
morgante
approved these changes
Oct 31, 2019
morgante
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
message_storage_policyto the topic #14