Skip to content

Conversation

Ram1604
Copy link
Contributor

@Ram1604 Ram1604 commented Mar 20, 2025

This feature is not added in the push subscription. This option is needed to enable or disable "Push payload unwrapping" from the module itself.

@Ram1604 Ram1604 requested review from a team, ayushmjain, imrannayer and q2w as code owners March 20, 2025 11:20
@Ram1604 Ram1604 changed the title Feat: Adding no wrapper option to Push subscription feat: Adding no wrapper option to Push subscription Mar 20, 2025
@Ram1604 Ram1604 changed the title feat: Adding no wrapper option to Push subscription feat: Add no wrapper option to Push subscription Mar 20, 2025
@Ram1604 Ram1604 changed the title feat: Add no wrapper option to Push subscription Feat: Add missing attribute no_wrapper option to push subscription Mar 20, 2025
@Ram1604
Copy link
Contributor Author

Ram1604 commented Mar 20, 2025

Hi @imrannayer @ayushmjain @q2w

Can you please check on this?

@Ram1604
Copy link
Contributor Author

Ram1604 commented Mar 22, 2025

Hi @imrannayer @ayushmjain @q2w 

Can you check on this?

@Ram1604
Copy link
Contributor Author

Ram1604 commented Mar 24, 2025

Hi @imrannayer @ayushmjain @q2w

Following up on this.

@Ram1604
Copy link
Contributor Author

Ram1604 commented Mar 25, 2025

Hi @imrannayer @q2w @ayushmjain

When can I expect this to be reviewed?

@imrannayer
Copy link
Collaborator

/gcbrun

@Ram1604
Copy link
Contributor Author

Ram1604 commented Mar 26, 2025

@imrannayer

Thanks for running the build. When can I expect this to be merged?

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @Ram1604!

  • Verified no_wrapper is present in TPG v6.2
  • Need to verify the metadata.yaml formatting prior to merge

filter = optional(string),
enable_message_ordering = optional(bool),
}))
varType: "list(object({\r\n name = string,\r\n ack_deadline_seconds = optional(number),\r\n push_endpoint = optional(string),\r\n x-goog-version = optional(string),\r\n oidc_service_account_email = optional(string),\r\n audience = optional(string),\r\n expiration_policy = optional(string),\r\n dead_letter_topic = optional(string),\r\n retain_acked_messages = optional(bool),\r\n message_retention_duration = optional(string),\r\n max_delivery_attempts = optional(number),\r\n maximum_backoff = optional(string),\r\n minimum_backoff = optional(string),\r\n filter = optional(string),\r\n enable_message_ordering = optional(bool),\r\n no_wrapper = optional(bool),\r\n write_metadata = optional(bool),\r\n }))"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen this formatting before, is this correct? @q2w?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ram1604 Hey, Did you use the make generate_docs to update the metadata.yaml?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@q2w I did ran it after that the metadata.yaml got updated.

@Ram1604
Copy link
Contributor Author

Ram1604 commented Mar 28, 2025

Hi @q2w

Can you kindly assist here?

@Ram1604
Copy link
Contributor Author

Ram1604 commented Mar 31, 2025

Hi @imrannayer @q2w @apeabody

Can you please assist here?

@q2w q2w merged commit 295df92 into terraform-google-modules:main Apr 1, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants