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

Don't expect aliases in output dictionaries #10921

Merged
merged 4 commits into from
Feb 28, 2019

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Feb 24, 2019

With the creation of aliases for ECS we have found that some features
weren't being migrated and were writing to aliases (like in #10757).
Consider writing to an alias field like writing to an undocumented field.

@jsoriano jsoriano marked this pull request as ready for review February 25, 2019 12:31
@jsoriano jsoriano requested a review from a team as a code owner February 25, 2019 12:31
@jsoriano
Copy link
Member Author

jsoriano commented Feb 25, 2019

Failing tests will pass once #10927 is merged, will rebase then.

@jsoriano jsoriano requested a review from a team February 26, 2019 09:49
Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Code seems ok although I'm not really sure about how to test it

@jsoriano
Copy link
Member Author

@sayden you can test it by writing from a module to a field documented as alias, and running the system tests for this module. For example if you revert #10927 and run test_docker.py you will see that docker module was creating events with docker.container.* fields, that are now defined as aliases to container.*.

I can wait to merge this if you want to give it a try.

@sayden
Copy link
Contributor

sayden commented Feb 28, 2019

I don't want to block this PR. Just wanted to give you some help trying a local test.

@jsoriano jsoriano merged commit 2585f61 into elastic:master Feb 28, 2019
@jsoriano jsoriano deleted the no-aliases-in-output branch February 28, 2019 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants