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

Glossary update, Closes Issue #16891 #29127

Merged
merged 10 commits into from
Apr 16, 2018
Merged

Conversation

refactormyself
Copy link
Contributor

Glossary Terms "Filter" and "Query" were added with details as described here https://www.elastic.co/guide/en/elasticsearch/guide/current/_queries_and_filters.html

@elasticmachine
Copy link
Collaborator

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

1 similar comment
@elasticmachine
Copy link
Collaborator

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

Copy link
Contributor Author

@refactormyself refactormyself left a comment

Choose a reason for hiding this comment

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

License now signed

@elasticmachine
Copy link
Collaborator

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

1 similar comment
@elasticmachine
Copy link
Collaborator

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

@jasontedor jasontedor added the :Search/Search Search-related issues that do not fall into other categories label Mar 18, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@jasontedor jasontedor added the >docs General docs changes label Mar 18, 2018
but also calculate how well the document matches. This calculation is refered
to as scoring, hence these queries are also known as "scoring queries".
A scoring query calculates how relevant each document is to the query, and assigns
it a relevance _score, which is later used to sort matching documents by relevance.
Copy link
Member

Choose a reason for hiding this comment

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

nit: use backticks for _score

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Hi @refactormyself,

thanks for picking up the issue and submitting this PR, greatly appreciated. I left a couple of small comments where I think the phrasing could be improved a bit but please take these only as suggestions. Since this is going to end up in the glossary I think its worth to try to find a good, concise definition of the terms, so hope you don't mind some of the comments. Also appreciate anybody elses opinions on this.

@@ -61,6 +61,16 @@
`object`. The mapping also allows you to define (amongst other things)
how the value for a field should be analyzed.

[[glossary-filter]] filter ::

A filter is kind of query known as a "non-scoring" query. It does not give a score,
Copy link
Member

Choose a reason for hiding this comment

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

This is correct, but could you make it clearer from the beginning that a "filter" is a normal query, just one that doesn't score? You already say this, but in the current order the definition isn't as clear as it could be. I think this just needs a little rephrasing.

Copy link
Member

Choose a reason for hiding this comment

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

also: "is a kind" if you leave that part in anywhere.


A filter is kind of query known as a "non-scoring" query. It does not give a score,
it is only concerned about answering the question - "Does this document match?".
The answer is always a simple, binary yes|no. This kind of query is said to be made
Copy link
Member

Choose a reason for hiding this comment

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

In the context of the glossary I would spell "yes|no" out and say yes or no.

A filter is kind of query known as a "non-scoring" query. It does not give a score,
it is only concerned about answering the question - "Does this document match?".
The answer is always a simple, binary yes|no. This kind of query is said to be made
in a "filtering" context, hence it is called a filter. Filtering queries/Filters are
Copy link
Member

Choose a reason for hiding this comment

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

Maybe explain "filtering" context, e.g. the "filter" section in a boolean query.

Copy link
Member

Choose a reason for hiding this comment

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

Why the "/" here? I'd simply talk about "filters" here.

Copy link
Contributor Author

@refactormyself refactormyself Mar 21, 2018

Choose a reason for hiding this comment

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

"Filtering queries/Filters" is used to emphasis that Filtering queries = Filters. Maybe I should make it - "Filtering queries or Filters"

it is only concerned about answering the question - "Does this document match?".
The answer is always a simple, binary yes|no. This kind of query is said to be made
in a "filtering" context, hence it is called a filter. Filtering queries/Filters are
simple checks for set inclusion/exclusion. The goal of filtering is to reduce the
Copy link
Member

Choose a reason for hiding this comment

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

"inclusion or exclusion"

in a "filtering" context, hence it is called a filter. Filtering queries/Filters are
simple checks for set inclusion/exclusion. The goal of filtering is to reduce the
number of documents that have to be examined, as it is in the case of
<<glossary-query,scoring queries>>.
Copy link
Member

Choose a reason for hiding this comment

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

I don't undestand the reference to scoring queries in this sentence, maybe you can explain or rephrase to make this clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal of filtering is to reduce the number of documents that have to be examined, instead of what happens in the case of <<glossary-query,scoring queries>>.

The reference is also meant to provide a link to the reader for comparism

@@ -105,6 +115,19 @@
+
See also <<glossary-routing,routing>>

[[glossary-query]] query ::

Query refers to all queries which not only determine if a document matches,
Copy link
Member

Choose a reason for hiding this comment

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

You start by contrasting queries with something else (filters I guess), but maybe its possible to start the definition in general? Maybe talk about queries as that what defines a search request and then later differentiate between queries that score and queries that don't score (filters)? Not sure about the best phrasing for this, I think the current formulations are already quiete good, maybe also just change the ordering a bit?

it a relevance _score, which is later used to sort matching documents by relevance.
This concept of relevance is well suited to full-text search, where there is seldom a
completely “correct” answer. These queries are more heavier than
<<glossary-filter,filters/non scoring queries>> and their query results are not cacheable.
Copy link
Member

Choose a reason for hiding this comment

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

I would either use "filters" or "non-scoring queries" here, probably the later.

A scoring query calculates how relevant each document is to the query, and assigns
it a relevance _score, which is later used to sort matching documents by relevance.
This concept of relevance is well suited to full-text search, where there is seldom a
completely “correct” answer. These queries are more heavier than
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if "more heavier" is correct (I'm not a native speaker) but it sounds like using "heavier" would be enough. Maybe "heavier" could also be exchanged for something that makes it clearer that we are talking about performance here.

@refactormyself
Copy link
Contributor Author

@cbuescher I hope I have addressed your comments

@cbuescher cbuescher self-assigned this Mar 28, 2018
@@ -61,6 +61,16 @@
`object`. The mapping also allows you to define (amongst other things)
how the value for a field should be analyzed.

[[glossary-filter]] filter ::

A filter is a query. It is a kind of query which does not give a score, so it is
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can join the first two sentences to make it more fluent, e.g. "A filter is a <<glossary-query,scoring queries>> that ..." and then join the second sentence.
Maybe use something like "produces a score" or "scores documents" instead of "give a score"?

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

@refactormyself thanks for the changes, I did another round of review and left yet another set of comments.

The PR looks great in general, sorry if the comments I left feel a bit picky, but I'd like to be a bit more careful about definitions in the glossary. Maybe I'll also ping another member of the team to take another look if they agree with me and to get another pair of opinions.

[[glossary-filter]] filter ::

A filter is a query. It is a kind of query which does not give a score, so it is
known as a "non-scoring" query.It is only concerned about answering the question -
Copy link
Member

Choose a reason for hiding this comment

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

nit: whitespace between "query.It". Although the difference won't show up in the docs I think, it improves readability here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @cbuescher I will work on your review. As a beginner to Elasticsearch myself, I appreciate the importance of getting the glossary right. I'll be happy to get it right.


A filter is a query. It is a kind of query which does not give a score, so it is
known as a "non-scoring" query.It is only concerned about answering the question -
"Does this document match?". The answer is always a simple, binary yes or no. This kind of query is said to be made
Copy link
Member

Choose a reason for hiding this comment

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

Could you wrap this (and the following lines) roughly at the same position as the previous lines? We have different line lengths in the docs throughout the documentation but at least in one paragraph it would be nice if it was roughly similar. Maybe it makes sense to do the wrapping in the end just before the PR is ready to be merged though, to just go through the trouble once.

A filter is a query. It is a kind of query which does not give a score, so it is
known as a "non-scoring" query.It is only concerned about answering the question -
"Does this document match?". The answer is always a simple, binary yes or no. This kind of query is said to be made
in a "filtering" context (e.g. Is the created date in the range 2013 - 2014?), hence it is called a filter. Filters are
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should link to https://www.elastic.co/guide/en/elasticsearch/reference/current/query-filter-context.html when mentioning the filter context. Also it would be great if it would be clearer that "filter context" defines specific locations of a query clause in a combination of queries. If this is too much detail for the glossary, I think linking to the "filter context" paragraph and removing the details here is fine as well.

known as a "non-scoring" query.It is only concerned about answering the question -
"Does this document match?". The answer is always a simple, binary yes or no. This kind of query is said to be made
in a "filtering" context (e.g. Is the created date in the range 2013 - 2014?), hence it is called a filter. Filters are
simple checks for set inclusion or exclusion. The goal of filtering is to reduce the
Copy link
Member

Choose a reason for hiding this comment

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

"The goal of filtering is to reduce the number of documents"

This might be true most of the time, but e.g. constant_score isn't really about reducing the document set, its just about saying you don't care about the score. I'd be okay with adding "most of the time" somewhere, maybe also leave out the "instead of..." part, since reducing document set size might also happen in scoring queries.

[[glossary-query]] query ::

A query is the basic component of a search. A search can be defined by one or more queries
which can be mixed and matched in endless combinations. The term Query refers to all queries
Copy link
Member

Choose a reason for hiding this comment

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

I'd scratch this statement, I think of "Query" as a superset of both "scoring queries" and filters, so I wouldn't introduce a distinction here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please, I will like to clarify this. So you mean whenever the term Query is used it can either mean "filters" or "scoring queries". So, unless it is clarified it can be either (or I guess, even both).

Copy link
Member

Choose a reason for hiding this comment

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

Others might disagree, but my understanding is that we only have "queries" in elasticsearch now (there used to be special filters in earlier versions). Some queries, if used in a "filtering context" as described in the link mentioned above, are also refered to as "filters" or "non-scoring queries" as oposed to "scoring queries" (when the queries are producing scores for documents). Whether something is a filter or not depends on their location. e.g. a term_query is generally speaking alway a query, but when it is used e.g. in a boolean queries filter clause it is also refered to as a filter in our documentation.

Copy link
Member

Choose a reason for hiding this comment

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

The above explanation might be a bit verbose for the glossary however, so making it a bit more succinct would be the goal here.


A query is the basic component of a search. A search can be defined by one or more queries
which can be mixed and matched in endless combinations. The term Query refers to all queries
which not only determine if a document matches, but also calculate how well the document matches.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe say "Queries that calculate how well the document matches are also known as "scoring queries", as oposed to queries that only determine if a document matches, which are also called <<glossary-filter,filters>>.

which can be mixed and matched in endless combinations. The term Query refers to all queries
which not only determine if a document matches, but also calculate how well the document matches.
This calculation is refered to as scoring, hence these queries are also known as "scoring queries".
A scoring query calculates how relevant each document is to the query, and assigns
Copy link
Member

Choose a reason for hiding this comment

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

I would leave out the notion of "relevance" here, this is opening yet another term that we'd need to refine more carefully I think. I would suggest using "how well a document mathces a query", "score" and "sort by score" instead.

This calculation is refered to as scoring, hence these queries are also known as "scoring queries".
A scoring query calculates how relevant each document is to the query, and assigns
it a relevance score, which is later used to sort matching documents by relevance.
This concept of relevance is well suited to full-text search, where there is seldom a
Copy link
Member

Choose a reason for hiding this comment

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

Just leave out "concept of relevance" if you agree with my former proposal.

A scoring query calculates how relevant each document is to the query, and assigns
it a relevance score, which is later used to sort matching documents by relevance.
This concept of relevance is well suited to full-text search, where there is seldom a
completely “correct” answer. These queries are takes more resources than
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use "Scoring queries" at the beginning of this sentence here.

completely “correct” answer. These queries are takes more resources than
<<glossary-filter,non scoring queries>> and their query results are not cacheable.
As a general rule, use query clauses for full-text search or for any condition that should
affect the relevance score, and use filters for everything else.
Copy link
Member

Choose a reason for hiding this comment

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

maybe user "condition that requires scoring" if you agree on leaving you the notion of relevance here.

@refactormyself
Copy link
Contributor Author

@cbuescher I made the changes, please review

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

@refactormyself thanks a lot, this looks quiet good to me already. I left a couple of small remarks still for various style reasons and to avoid repetitions, but the two sections are getting close to being ready I think.

@@ -54,19 +54,26 @@
pairs. The value can be a simple (scalar) value (eg a string, integer,
date), or a nested structure like an array or an object. A field is
similar to a column in a table in a relational database.
+
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to keep these lines in (also please revert the following deletes). They prevent paragraph breaks but make the source file more readable.

Copy link
Contributor Author

@refactormyself refactormyself Mar 29, 2018

Choose a reason for hiding this comment

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

Sorry, please I don't understand your point here

Copy link
Member

Choose a reason for hiding this comment

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

The point was that the added free line in the documentation source is more readable as it separates the following paragraph visually, while the "+" prevents this from happening in the final page. I think this was intentional and shouldn't be changed as part of this PR, so please revert these changes.

A filter is a non-scoring <<glossary-query,query>>, meaning that it does not score documents.
It is only concerned about answering the question - "Does this document match?".
The answer is always a simple, binary yes or no. This kind of query is said to be made
in a https://www.elastic.co/guide/en/elasticsearch/reference/current/query-filter-context.html["filtering" context],
Copy link
Member

Choose a reason for hiding this comment

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

I think you can use a relative link like <<query-filter-context,filter context>> here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried it is not working

Copy link
Member

Choose a reason for hiding this comment

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

I also just tried, works for me. What are you doing to check the resulting page? Have you tried building the whole reference doc, e.g. by using something like build_docs.pl --doc docs/reference/index.asciidoc --out ~/temp/asciidoc/ --open where build_docs.pl points to my local checkout of https://github.com/elastic/docs?


A query is the basic component of a search. A search can be defined by one or more queries
which can be mixed and matched in endless combinations. While <<glossary-filter,filters>> are
queries that only determine if a document matches, those queries that also calculate or score
Copy link
Member

Choose a reason for hiding this comment

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

"calculate or score" sound like two separate things, I'd just use one, probably prefer "... also calculate how wel..."

A query is the basic component of a search. A search can be defined by one or more queries
which can be mixed and matched in endless combinations. While <<glossary-filter,filters>> are
queries that only determine if a document matches, those queries that also calculate or score
how well the document matches are known as "scoring queries". A scoring query calculates how
Copy link
Member

Choose a reason for hiding this comment

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

I'd scratch "calculates how well a document matches a query" here becauce this is already mentioned in the previous sentence. Maybe just use "Those queries assigns a score, which ..." and also leave out "... by score" at the end, too much repetition.

which can be mixed and matched in endless combinations. While <<glossary-filter,filters>> are
queries that only determine if a document matches, those queries that also calculate or score
how well the document matches are known as "scoring queries". A scoring query calculates how
well a document mathces a query, and assigns it a score, which is later used to sort matching
Copy link
Member

Choose a reason for hiding this comment

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

"matches", but not necessary if you scratch the whole part

@refactormyself
Copy link
Contributor Author

@cbuescher hope I placed the "+" right back

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

@refactormyself thanks for the last round of updates. I like the text much better now, I think the only think left is trying to revert all the remaining formatting changes not related to the two new sections and maybe try using the relative link again. Let me know if you have issues with that, I tried it and it works for me as I mentioned in the comment.

@@ -54,19 +54,26 @@
pairs. The value can be a simple (scalar) value (eg a string, integer,
date), or a nested structure like an array or an object. A field is
similar to a column in a table in a relational database.
+
Copy link
Member

Choose a reason for hiding this comment

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

The point was that the added free line in the documentation source is more readable as it separates the following paragraph visually, while the "+" prevents this from happening in the final page. I think this was intentional and shouldn't be changed as part of this PR, so please revert these changes.

@@ -128,7 +146,6 @@
the ID of the document or, if the document has a specified parent
document, from the ID of the parent document (to ensure that child and
parent documents are stored on the same shard).
+
Copy link
Member

Choose a reason for hiding this comment

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

please also revert this change

@@ -143,7 +160,6 @@
Other than defining the number of primary and replica shards that an
index should have, you never need to refer to shards directly.
Instead, your code should deal only with an index.
+
Copy link
Member

Choose a reason for hiding this comment

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

please also revert this change

A filter is a non-scoring <<glossary-query,query>>, meaning that it does not score documents.
It is only concerned about answering the question - "Does this document match?".
The answer is always a simple, binary yes or no. This kind of query is said to be made
in a https://www.elastic.co/guide/en/elasticsearch/reference/current/query-filter-context.html["filtering" context],
Copy link
Member

Choose a reason for hiding this comment

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

I also just tried, works for me. What are you doing to check the resulting page? Have you tried building the whole reference doc, e.g. by using something like build_docs.pl --doc docs/reference/index.asciidoc --out ~/temp/asciidoc/ --open where build_docs.pl points to my local checkout of https://github.com/elastic/docs?

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Almost there ;-). The link doesn't work yet, unfortunately, I left a comment how to fix it. After that I think this is ready to get merged. Thanks for sticking with this until now.

A filter is a non-scoring <<glossary-query,query>>, meaning that it does not score documents.
It is only concerned about answering the question - "Does this document match?".
The answer is always a simple, binary yes or no. This kind of query is said to be made
in a <<https://www.elastic.co/guide/en/elasticsearch/reference/current/query-filter-context.html,"filtering" context>>,
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately this way of adding the link doesn't work, when I build the page I get a link validity error. You should use a link Id like in the other cases in this paragraph. In this case <<query-filter-context,filter context>> should work. I would also leave out the quotes and change the link text to "filter context" since that is the what it is called in the page that is refered to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check it and see now, but it is not looking good on my end

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by "not looking good on my end"? The current update still contains the absolute link and that will break our docs build, so this will need changing unfortunately. What do you run currently to check if the docs are built correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is okay now, I didn't pay attention to what I doing wrong until you mentioned about the "absolute link" again. Hope is fixed now.

@@ -66,7 +66,7 @@
A filter is a non-scoring <<glossary-query,query>>, meaning that it does not score documents.
It is only concerned about answering the question - "Does this document match?".
The answer is always a simple, binary yes or no. This kind of query is said to be made
in a <<https://www.elastic.co/guide/en/elasticsearch/reference/current/query-filter-context.html,filter context>>,
in a <<query-dsl/query_filter_context#,filter context>>,
Copy link
Member

Choose a reason for hiding this comment

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

This still gives an error when building the full reference docs (e.g. using build_docs.pl --doc docs/reference/index.asciidoc --out ~/temp/asciidoc/ --open):

/Users/xxx/temp/asciidoc/index.xml:78053: element link: validity error : Syntax of value for attribute linkend of link is not valid
  in a <link linkend="query-dsl/query_filter_context#">filter context</link>,
                                                      ^
/Users/xxx/temp/asciidoc/index.xml:78053: element link: validity error : IDREF attribute linkend references an unknown ID "query-dsl/query_filter_context#"

Which is why I was asking you earlier how you build the docs and what errors you get with the version I suggested (<<query-filter-context,filter context>>). Can you either tell me that so I can help or simply use that link instead? .

@cbuescher
Copy link
Member

@refactormyself sorry, I'm still having trouble with that link. Have you built the docs yourself? If so, how? What was the problem with the version I suggested?

@cbuescher
Copy link
Member

@refactormyself any news here? I think this is very close, needs only the link to work correctly.

@refactormyself
Copy link
Contributor Author

@cbuescher I had been previously confirming the correctness of glossary.asciidoc by individually previewing the generated html. That is why I would not be getting the error you are talking about. However, I built the whole elasticsearch project and got no error.

When you talked about the error I tried to build as directed but I got stuck, I just can't correlate the link between https://github.com/elastic/elasticsearch/tree/master/docs and this docs repo here https://github.com/elastic/docs.

My guess is that the latter is generated from the former, but how? I am definitely missing out something. Please help.

@cbuescher
Copy link
Member

I just can't correlate the link between https://github.com/elastic/elasticsearch/tree/master/docs and this docs repo here https://github.com/elastic/docs.

My guess is that the latter is generated from the former, but how? I am definitely missing out something. Please help.

Sure, happy to help.
The first url you mentioned (https://github.com/elastic/elasticsearch/tree/master/docs) points to the elasticsearch documentation source files that are kept together with the code in the main elasticsearch open source repository. This is where you already made changes to the glossary.asciidoc document.
The second location (https://github.com/elastic/docs) points to a repository that contains both tools for building the documentation as well as some of the artifacts produced by that build process. That why there's so much html stuff in there.
To check if changes you made work, please refer to the README.asciidoc in the later repo. For example, I have this repo checked out locally and have a shortcut link pointing to the build_docs.pl script that is located in the root folder of that repo. When I e.g. try to build the glossary page alone locally I do build_docs.pl --doc docs/reference/glossary.asciidoc --out ~/temp/asciidoc/ --open --lenient. --open opens the document in the browser once build and --lenient supresses the link errors that I get otherwise because I'm only building a single page. With this you won't be able to check the link works correctly (because of the --lenient) but it helps in earlier phases of making small changes.
In the case of your PR, to check the link works you probably need to build the whole reference book by using something like build_docs.pl --doc docs/reference/index.asciidoc --out ~/temp/asciidoc/ --open. This might take a few minutes (e.g. approx. 3 minutes on my laptop) but will resolve links etc... This is where I get the above mentioned error when building from your PR branch. If you use <<query-filter-context,filter context>> instead of the current link, the link works when building the reference docs locally.

If you have trouble verifying using any of the above steps please let me know, otherwise I think this PR only needs correction of that "filter context" link before merging.

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

I think I see why the way document you are trying to link only works with an external link. I'd prefer to link a different page in reference documentation itself.

A filter is a non-scoring <<glossary-query,query>>, meaning that it does not score documents.
It is only concerned about answering the question - "Does this document match?".
The answer is always a simple, binary yes or no. This kind of query is said to be made
in a https://www.elastic.co/guide/en/elasticsearch/guide/current/_queries_and_filters.html[filter context],
Copy link
Member

Choose a reason for hiding this comment

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

I think I see your problem now.
You are trying to link to a page in the elasticsearch guide, which only seems to work with an external link syntax. What I said earlier was to please use an "Internal cross references" as described in the asciidoc syntax above to point to this document in the reference itself, which works because of its inline anchor in the first line. You should be able to use <<query-filter-context,filter context>> for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried even this <<query_filter_context.asciidoc,filter context>> before, now again, it is not working. It only works if both the linked and linking documents are in the same folder. Here we have docs/reference/glossary.asciidoc linking to docs/reference/query-dsl/query_filter_context.asciidoc. I found this issue on asciidoctor repo but I could not make anything of it. I tried the follow with no success :-
<</query-dsl/query_filter_context.asciidoc,filter context>>
<<./query-dsl/query_filter_context.asciidoc,filter context>>
<<../query-dsl/query_filter_context.asciidoc,filter context>>

I build this before pushing and it did not break any test. Must all references follow the <<,>> syntax ? Sorry, please I don't seem to get the problem, in this case.

Copy link
Member

@cbuescher cbuescher Apr 13, 2018

Choose a reason for hiding this comment

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

Have you tried the version I gave you (<<query-filter-context,filter context>>) using the build_docs.pl script I outlined above? All the variant you just listed are different.

I build this before pushing and it did not break any test

I'm still trying to get at what you mean by that. Could you explain what tooling you use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This <<query-filter-context,filter context>> gives me this:

/temp/asciidoc/index.xml:78053: element link: validity error : IDREF attribute linkend references an unknown ID "query_filter_context"

I am on Windows 7, using CGwin - bash, perl, python, xsltproc installed

elasticdocs

Copy link
Member

Choose a reason for hiding this comment

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

Can you please push the version where you get that error?

@@ -66,7 +66,7 @@
A filter is a non-scoring <<glossary-query,query>>, meaning that it does not score documents.
It is only concerned about answering the question - "Does this document match?".
The answer is always a simple, binary yes or no. This kind of query is said to be made
in a https://www.elastic.co/guide/en/elasticsearch/guide/current/_queries_and_filters.html[filter context],
in a <<query_filter_context,filter context>>,
Copy link
Member

Choose a reason for hiding this comment

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

Use <<query-filter-context,filter context>>, the first needs to be the anchor tag of the target section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I did it now. I guess it's working now. Sorry, I should have paid more attention to the docs. Thanks

Copy link
Member

Choose a reason for hiding this comment

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

I'm glad this works on your side now as well.

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

@refactormyself thanks for the latest change, this looks ready now. I'll merge this to the currently active branches now. I think it is a great addition to the glossary.

@cbuescher cbuescher merged commit 22af392 into elastic:6.2 Apr 16, 2018
cbuescher pushed a commit that referenced this pull request Apr 16, 2018
Definitions for "filter" and "query" are added to the glossary of terms.

Closes #29127
cbuescher pushed a commit that referenced this pull request Apr 16, 2018
Definitions for "filter" and "query" are added to the glossary of terms.

Closes #29127
cbuescher pushed a commit that referenced this pull request Apr 16, 2018
Definitions for "filter" and "query" are added to the glossary of terms.

Closes #29127
cbuescher pushed a commit that referenced this pull request Apr 16, 2018
Definitions for "filter" and "query" are added to the glossary of terms.

Closes #29127
jasontedor added a commit that referenced this pull request Apr 16, 2018
* 6.x:
  [TEST] REST client request without leading '/' (#29471)
  Prevent accidental changes of default values (#29528)
  [Docs] Add definitions to glossary  (#29127)
  Avoid self-deadlock in the translog (#29520)
  Minor cleanup in NodeInfo.groovy
  Lazy configure build tasks that require older JDKs (#29519)
  Fix usage of NodeInfo#nodeVersion in build
  Control max size/count of warning headers (#28427) (#29516)
  Simplify snapshot check in root build file
  Make NodeInfo#nodeVersion strongly-typed as Version (#29515)
  Enable license header exclusions (#29379)
  Use proper Java version for BWC builds (#29493)
  Mute TranslogTests#testFatalIOExceptionsWhileWritingConcurrently
jasontedor added a commit that referenced this pull request Apr 16, 2018
* master:
  [TEST] REST client request without leading '/' (#29471)
  Using ObjectParser in UpdateRequest (#29293)
  Prevent accidental changes of default values (#29528)
  [Docs] Add definitions to glossary  (#29127)
  Avoid self-deadlock in the translog (#29520)
  Minor cleanup in NodeInfo.groovy
  Lazy configure build tasks that require older JDKs (#29519)
  Simplify snapshot check in root build file
  Make NodeInfo#nodeVersion strongly-typed as Version (#29515)
  Enable license header exclusions (#29379)
  Use proper Java version for BWC builds (#29493)
  Mute TranslogTests#testFatalIOExceptionsWhileWritingConcurrently
  Enable skipping fetching latest for BWC builds (#29497)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 17, 2018
* master: (96 commits)
  TEST: Mute testEnsureWeReconnect
  Mute full cluster restart test recovery test
  REST high-level client: add support for Indices Update Settings API [take 2] (elastic#29327)
  Plugins: Fix native controller confirmation for non-meta plugin (elastic#29434)
  Remove PipelineExecutionService#executeIndexRequest (elastic#29537)
  Fix overflow error in parsing of long geohashes (elastic#29418)
  Remove unused index.ttl.disable_purge setting (elastic#29527)
  FullClusterRestartIT.testRecovery should wait for all initializing shards
  Build: Fail if any libs depend on non-core libs (elastic#29336)
  [TEST] REST client request without leading '/' (elastic#29471)
  Using ObjectParser in UpdateRequest (elastic#29293)
  Prevent accidental changes of default values (elastic#29528)
  [Docs] Add definitions to glossary  (elastic#29127)
  Avoid self-deadlock in the translog (elastic#29520)
  Minor cleanup in NodeInfo.groovy
  Lazy configure build tasks that require older JDKs (elastic#29519)
  Simplify snapshot check in root build file
  Make NodeInfo#nodeVersion strongly-typed as Version (elastic#29515)
  Enable license header exclusions (elastic#29379)
  Use proper Java version for BWC builds (elastic#29493)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Search/Search Search-related issues that do not fall into other categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants