Skip to content

Update migration file to new structure#9381

Merged
ruflin merged 1 commit intoelastic:masterfrom
ruflin:migration-file-update
Dec 5, 2018
Merged

Update migration file to new structure#9381
ruflin merged 1 commit intoelastic:masterfrom
ruflin:migration-file-update

Conversation

@ruflin
Copy link
Copy Markdown
Contributor

@ruflin ruflin commented Dec 4, 2018

The new structure makes it clear which alias have to happen in 6.x and which ones in 7.x

@ruflin ruflin added review ecs Team:Integrations Label for the Integrations team labels Dec 4, 2018
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/infrastructure

@ruflin ruflin force-pushed the migration-file-update branch from 160bda9 to 388e342 Compare December 4, 2018 14:50
@ruflin ruflin requested review from graphaelli and webmat December 4, 2018 14:51
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

many-1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see that fix. Have you pushed? 😂

Copy link
Copy Markdown
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

LGTM. Just a nit about adding a comment for the meaning when alias6 is absent

Copy link
Copy Markdown
Contributor Author

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

@webmat would be great if you could go through the fields to make sure there are no mistakes inside.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@webmat
Copy link
Copy Markdown
Contributor

webmat commented Dec 4, 2018

Yeah I did read through quickly. Will pore through it one more time.

The new structure makes it clear which alias have to happen in 6.x and which ones in 7.x
@ruflin ruflin force-pushed the migration-file-update branch from 388e342 to 782a491 Compare December 4, 2018 15:10
copy_to: false

- from: beat.hostname
to: agent.hostname
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@MikePaquette is pushing back on agent.hostname, btw.

Agent should always be running on the host itself. If the monitoring agent is outside of the host, it should be represented in observer.hostname (used to be called device.hostname, but device is being renamed to observer).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be addressed in another PR, though.

@webmat webmat self-requested a review December 4, 2018 15:20
Copy link
Copy Markdown
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

I went through the file again. The mappings look good, but I think the Auditbeat mapping should be removed.

There should be a pull request to address Auditbeat in its entirety.

@ruflin ruflin merged commit 2aca783 into elastic:master Dec 5, 2018
@ruflin ruflin mentioned this pull request Dec 5, 2018
@ruflin ruflin deleted the migration-file-update branch December 5, 2018 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ecs review Team:Integrations Label for the Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants