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

[Backport 2.x] [Star tree] Adding date field rounding support in star tree #16304

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

Backport 1e49aa8 from #15249.

---------

Signed-off-by: Bharathwaj G <[email protected]>
(cherry picked from commit 1e49aa8)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

Why isn't there a CHANGELOG entry for this (or even on the original main PR)? Seems a significant feature?

Copy link
Contributor

❌ Gradle check result for 33404fe: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@bharath-techie
Copy link
Contributor

Why isn't there a CHANGELOG entry for this (or even on the original main PR)? Seems a significant feature?

@dbwiddis In case of star tree changes, we have already added CHANGELOG entry to the core star tree mapper PR as it was user-facing. For all the other changes which were in backend in the indexing flow , we skipped adding changelog.

But this PR does have some user facing changes in the mapping, so makes sense to add CHANGELOG.

Can I add it in followup PR ? "Support for keyword fields as part of star tree mapping #16103" will be coming next, I can add CHANGELOG entries for both.

Copy link
Contributor

❌ Gradle check result for 33404fe: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@dbwiddis
Copy link
Member

But this PR does have some user facing changes in the mapping, so makes sense to add CHANGELOG.

Can I add it in followup PR ?

Sure, since you'll need to start on main anyway. Also be sure to add any doc/api changes as the PR template shows.

I understand not having a changelog for an unreleased feature, but changes/additions/bugfixes/refactors after the version in which it was released should almost always have a changelog.

Copy link
Contributor

✅ Gradle check result for 33404fe: SUCCESS

Copy link

codecov bot commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 92.68293% with 15 lines in your changes missing coverage. Please review.

Project coverage is 71.74%. Comparing base (5947002) to head (33404fe).
Report is 13 commits behind head on 2.x.

Files with missing lines Patch % Lines
...va/org/opensearch/index/mapper/StarTreeMapper.java 84.84% 2 Missing and 3 partials ⚠️
...h/index/compositeindex/datacube/ReadDimension.java 50.00% 2 Missing ⚠️
...index/datacube/startree/StarTreeIndexSettings.java 71.42% 1 Missing and 1 partial ⚠️
...datacube/startree/builder/BaseStarTreeBuilder.java 95.55% 1 Missing and 1 partial ⚠️
...acube/startree/utils/date/DateTimeUnitAdapter.java 80.00% 0 Missing and 2 partials ⚠️
...ndex/compositeindex/datacube/DimensionFactory.java 83.33% 0 Missing and 1 partial ⚠️
...ompositeindex/datacube/startree/StarTreeField.java 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##                2.x   #16304    +/-   ##
==========================================
  Coverage     71.74%   71.74%            
- Complexity    65022    65031     +9     
==========================================
  Files          5302     5305     +3     
  Lines        303866   303969   +103     
  Branches      44192    44207    +15     
==========================================
+ Hits         218012   218096    +84     
- Misses        67648    67691    +43     
+ Partials      18206    18182    -24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@linuxpi linuxpi merged commit c7b865e into 2.x Oct 15, 2024
64 of 66 checks passed
@github-actions github-actions bot deleted the backport/backport-15249-to-2.x branch October 15, 2024 05:53
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.

3 participants