Skip to content

Conversation

@asimmahmood1
Copy link
Contributor

@asimmahmood1 asimmahmood1 commented Feb 17, 2025

Description

This PR adds documentation for execution_hint field that is being added for cardinality aggregator introduced in opensearch-project/OpenSearch#17312. Waiting for main repo PR to be merged.

This PR updates #8265

Issues Resolved

Closes #8264

Version

3.0

Frontend features

n/a

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions
Copy link

Thank you for submitting your PR. The PR states are In progress (or Draft) -> Tech review -> Doc review -> Editorial review -> Merged.

Before you submit your PR for doc review, make sure the content is technically accurate. If you need help finding a tech reviewer, tag a maintainer.

When you're ready for doc review, tag the assignee of this PR. The doc reviewer may push edits to the PR directly or leave comments and editorial suggestions for you to address (let us know in a comment if you have a preference). The doc reviewer will arrange for an editorial review.

@kolchfa-aws
Copy link
Collaborator

@asimmahmood1 FYI: There is another PR on this topic here #8265.

@asimmahmood1
Copy link
Contributor Author

@asimmahmood1 FYI: There is another PR on this topic here #8265.

Yes, I've kept that original commit and add mine on top. I wanted to make sure to provide the warning.

@kolchfa-aws
Copy link
Collaborator

Thanks! So should we close the original PR because this one contains the original commit?

@asimmahmood1
Copy link
Contributor Author

asimmahmood1 commented Feb 18, 2025

Thanks! So should we close the original PR because this one contains the original commit?

Yes, we can close the original PR. Note: I don't have permission to.

@kolchfa-aws
Copy link
Collaborator

@asimmahmood1 Is this PR ready for doc review?

@asimmahmood1
Copy link
Contributor Author

asimmahmood1 commented Feb 20, 2025

Code PR opensearch-project/OpenSearch#17312 has been merged, so this is ready for merge review.

Signed-off-by: Fanit Kolchina <[email protected]>
@kolchfa-aws
Copy link
Collaborator

@asimmahmood1 Doc review complete. Moving this PR to editorial review. Is this still slated for 3.0, or is this going into 2.19.1?

@asimmahmood1
Copy link
Contributor Author

asimmahmood1 commented Feb 24, 2025

Thank you @kolchfa-aws . Its been merged into 2.19.1 as well, and 2.x PR is also open.

Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

@kolchfa-aws @asimmahmood1 Just one minor change. Thanks!

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Asim M <[email protected]>
Copy link
Collaborator

@kolchfa-aws kolchfa-aws left a comment

Choose a reason for hiding this comment

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

LGTM

@kolchfa-aws kolchfa-aws added v2.19.1 Done but waiting to merge PR: The work is done and ready to merge backport 2.19 labels Feb 26, 2025
@asimmahmood1
Copy link
Contributor Author

Just to confirm, this should goto 2.19.1, 2.x branch and main/3.0 branch.

@kolchfa-aws kolchfa-aws merged commit 2d39e1c into opensearch-project:main Feb 27, 2025
9 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 27, 2025
* Execution hint documentation added to cardinality agg

Signed-off-by: Siddharth Rayabharam <[email protected]>

* Add expert level warning to cardinality execution_hint paramter

Signed-off-by: Asim Mahmood <[email protected]>

* Doc review

Signed-off-by: Fanit Kolchina <[email protected]>

* Update _aggregations/metric/cardinality.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Asim M <[email protected]>

---------

Signed-off-by: Siddharth Rayabharam <[email protected]>
Signed-off-by: Asim Mahmood <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Asim M <[email protected]>
Co-authored-by: Siddharth Rayabharam <[email protected]>
Co-authored-by: Fanit Kolchina <[email protected]>
Co-authored-by: kolchfa-aws <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
(cherry picked from commit 2d39e1c)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
epugh pushed a commit to o19s/documentation-website that referenced this pull request Jul 2, 2025
…ject#9224)

* Execution hint documentation added to cardinality agg

Signed-off-by: Siddharth Rayabharam <[email protected]>

* Add expert level warning to cardinality execution_hint paramter

Signed-off-by: Asim Mahmood <[email protected]>

* Doc review

Signed-off-by: Fanit Kolchina <[email protected]>

* Update _aggregations/metric/cardinality.md

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Asim M <[email protected]>

---------

Signed-off-by: Siddharth Rayabharam <[email protected]>
Signed-off-by: Asim Mahmood <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Asim M <[email protected]>
Co-authored-by: Siddharth Rayabharam <[email protected]>
Co-authored-by: Fanit Kolchina <[email protected]>
Co-authored-by: kolchfa-aws <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Eric Pugh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19 Done but waiting to merge PR: The work is done and ready to merge v2.19.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DOC] Documentation change for Cardinality Aggregator: execution_hint field added

4 participants