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

Add instance_name in GCE add_cloud_metadata processor #4414

Merged
merged 1 commit into from
May 30, 2017
Merged

Add instance_name in GCE add_cloud_metadata processor #4414

merged 1 commit into from
May 30, 2017

Conversation

BeLi4L
Copy link
Contributor

@BeLi4L BeLi4L commented May 29, 2017

No description provided.

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically on build-eu-00. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@tsg
Copy link
Contributor

tsg commented May 29, 2017

Thanks, can you sign the CLA, please? Should only take 5 mins.

One question on the "name" key. Is it guaranteed to always be there? Or should we mark it as optional?

@BeLi4L BeLi4L changed the title Add instance_name in GCE cloud processor Add instance_name in GCE add_cloud_metadata processor May 29, 2017
@BeLi4L
Copy link
Contributor Author

BeLi4L commented May 29, 2017

Yes, the "name" field is always there.

See https://cloud.google.com/compute/docs/reference/latest/instances#name

@tsg
Copy link
Contributor

tsg commented May 29, 2017

jenkins, test it

@ruflin
Copy link
Contributor

ruflin commented May 30, 2017

I think this also needs to be added to the fields.yml as it is not there yet.

@BeLi4L
Copy link
Contributor Author

BeLi4L commented May 30, 2017

Done ;)

@exekias
Copy link
Contributor

exekias commented May 30, 2017

@BeLi4L you will need to run make update in the root folder of the project, it will update docs with your new field.

Also please add a line to CHANGELOG.asciidoc file

Apart from that, LGTM :) thanks for contributing!

@BeLi4L
Copy link
Contributor Author

BeLi4L commented May 30, 2017

Alright, fixed :)

Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

Waiting for green

@exekias
Copy link
Contributor

exekias commented May 30, 2017

jenkins test this please

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.

5 participants