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

Rename url.href to url.original #109

Merged
merged 1 commit into from
Nov 1, 2018
Merged

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Sep 3, 2018

Recently we changed several fields which contain the initial original value to .original. href now follows the same example.

There is an issue with original in the description as it is text by default and we have .raw. What is our recommended approach here?

@webmat
Copy link
Contributor

webmat commented Sep 4, 2018

I would keep the keyword indexing but rename the nested field to url.original.keyword.

I think full text search on an original URL can be useful for ad hoc filtering on parts of the URL. So I would keep it around.

@webmat webmat mentioned this pull request Sep 18, 2018
26 tasks
@webmat webmat self-requested a review September 24, 2018 19:39
@ruflin ruflin force-pushed the href-original branch 2 times, most recently from a23730f to 78ab805 Compare October 31, 2018 13:37
@ruflin
Copy link
Member Author

ruflin commented Oct 31, 2018

@webmat Quite a few things changed since the initial PR. Could you have a look again?

Copy link
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 as is.

People who need full text indexing on the full URL can add it as a multi-field. I love this new convention :-)

Recently we changed several fields which contain the initial original value to .original. `href` now follows the same example.

There is an issue with original in the description as it is text by default and we have .raw. What is our recommended approach here?
@ruflin ruflin merged commit f255763 into elastic:master Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants