Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make nr_tags optional #75

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adiesner
Copy link

The documentation states that the field nr_tags is optional. But the
terraform code requires you to specify the field.
Documentation: https://docs.newrelic.com/docs/logs/forward-logs/aws-lambda-sending-cloudwatch-logs/

Error without setting the nr_tags field:

➜ terraform plan
╷
│ Error: Missing required argument
│
│   on main.tf line 21, in module "newrelic_log_ingestion":
│   21: module "newrelic_log_ingestion" {
│
│ The argument "nr_tags" is required, but no definition was found.

This commit adds a default value, so that the field is actually optional
as stated in the documentation.

The documentation states that the field nr_tags is optional. But the
terraform code requires you to specify the field.
Documentation: https://docs.newrelic.com/docs/logs/forward-logs/aws-lambda-sending-cloudwatch-logs/

Error without setting the nr_tags field:
➜ terraform plan
╷
│ Error: Missing required argument
│
│   on main.tf line 21, in module "newrelic_log_ingestion":
│   21: module "newrelic_log_ingestion" {
│
│ The argument "nr_tags" is required, but no definition was found.

This commit adds a default value, so that the field is actually optional
as stated in the documentation.
@CLAassistant
Copy link

CLAassistant commented Apr 28, 2023

CLA assistant check
All committers have signed the CLA.

@jponc
Copy link

jponc commented Jul 10, 2023

Hi @adiesner , do we know if this is getting merged anytime soon? You're right, this should be optional

@adiesner
Copy link
Author

Hi @adiesner , do we know if this is getting merged anytime soon? You're right, this should be optional

Sadly I have no idea. I never heard anything back from them besides an automated email from the CLAassistant. I will get in touch with my contact at NewRelic and ask what is missing.

@stevemao
Copy link

@jcsobrino please merge this PR.

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