fix: support for non-string types in object fields #11066

vignesh-hbk · 2024-08-01T07:03:40Z

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

Summary by CodeRabbit

New Features
- Enhanced handling of map-type values for improved data accuracy in search documents.
- Added support for various data types (boolean, long, float, double, integer) in TestEntityInfo, expanding its functionality.
Bug Fixes
- Improved type-specific value assignment to ensure correct data representation based on schema definitions.
Tests
- Increased test coverage with additional assertions to validate data types in the transformed JSON objects.
Documentation
- New fields in TestEntityInfo now support more complex data structures and are queryable.

coderabbitai · 2024-08-01T07:03:48Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The recent changes enhance the handling of various data types within the TestEntityInfo and SearchDocumentTransformer classes. New fields were added to TestEntityInfo to support boolean, integer, float, double, and long data types, improving its complexity and search capabilities. The SearchDocumentTransformer method was updated to ensure proper type parsing for map values based on schema definitions, while corresponding unit tests were enhanced to validate type correctness.

Changes

Files	Change Summary
`metadata-io/src/main/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformer.java`	Enhanced `setSearchableValue` method for better map-type value handling, adding type-specific parsing based on schema definitions.
`metadata-io/src/test/java/com/linkedin/metadata/TestEntityUtil.java`	Added new mappings in `TestEntityInfo` for various data types (boolean, long, float, double, integer), improving test scenarios with more complex structures.
`metadata-io/src/test/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformerTest.java`	Introduced additional assertions in `testTransform` to validate JSON field types (boolean, long, float, double, integer), enhancing test coverage for new data types.
`test-models/src/main/pegasus/com/datahub/test/TestEntityInfo.pdl`	Added new optional fields in `TestEntityInfo` for different data types (long, boolean, float, double, integer), expanding its structure and search capabilities.

Poem

🐇 In the forest where data blooms,
A rabbit hops 'midst code and tunes.
New fields sprout, each type a delight,
Transforming values, making them right.
With each change, our tales expand,
In the land of metadata, we make our stand! 🌼✨

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
-- I pushed a fix in commit <commit_id>, please review it.
-- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
-- @coderabbitai generate unit testing code for this file.
-- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
-- @coderabbitai generate interesting stats about this repository and render them as a table.
-- @coderabbitai read src/utils.ts and generate unit testing code.
-- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
-- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2ab43f3 and da12f1a.

Files selected for processing (4)

metadata-io/src/main/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformer.java (2 hunks)
metadata-io/src/test/java/com/linkedin/metadata/TestEntityUtil.java (2 hunks)
metadata-io/src/test/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformerTest.java (1 hunks)
test-models/src/main/pegasus/com/datahub/test/TestEntityInfo.pdl (1 hunks)

Additional comments not posted (16)

test-models/src/main/pegasus/com/datahub/test/TestEntityInfo.pdl (5)

107-114: LGTM!

The field esObjectFieldLong is correctly defined and annotated.

116-123: LGTM!

The field esObjectFieldBoolean is correctly defined and annotated.

125-132: LGTM!

The field esObjectFieldFloat is correctly defined and annotated.

134-141: LGTM!

The field esObjectFieldDouble is correctly defined and annotated.

143-150: LGTM!

The field esObjectFieldInteger is correctly defined and annotated.

metadata-io/src/test/java/com/linkedin/metadata/TestEntityUtil.java (5)

78-79: LGTM!

The mapping for esObjectFieldBoolean is correctly implemented.

80-81: LGTM!

The mapping for esObjectFieldLong is correctly implemented.

82-83: LGTM!

The mapping for esObjectFieldFloat is correctly implemented.

84-85: LGTM!

The mapping for esObjectFieldDouble is correctly implemented.

86-87: LGTM!

The mapping for esObjectFieldInteger is correctly implemented.

metadata-io/src/test/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformerTest.java (5)

88-88: LGTM!

The assertion for esObjectFieldBoolean is correctly implemented.

89-89: LGTM!

The assertion for esObjectFieldLong is correctly implemented.

90-90: LGTM!

The assertion for esObjectFieldFloat is correctly implemented.

91-91: LGTM!

The assertion for esObjectFieldDouble is correctly implemented.

92-92: LGTM!

The assertion for esObjectFieldInteger is correctly implemented.

metadata-io/src/main/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformer.java (1)

285-311: LGTM! Ensure the parsing logic is correct.

The changes to handle various data types in map values are well-implemented. Each data type is handled separately, ensuring correct parsing.

However, ensure that the parsing logic is correct and handles any potential exceptions.

...io/src/test/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformerTest.java

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between da12f1a and 63a8a5c.

Files selected for processing (1)

metadata-io/src/test/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformerTest.java (1 hunks)

Files skipped from review as they are similar to previous changes (1)

metadata-io/src/test/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformerTest.java

Nbagga14 · 2024-08-20T10:57:34Z

@david-leifker can you please review the above PR?

david-leifker · 2024-09-03T15:55:08Z

@Nbagga14 - Please run ./gradlew spotlessApply --rerun-tasks

vignesh-hbk · 2024-09-04T17:35:34Z

@david-leifker - I have formatted the code. Could you please review? Please let me know if anything further needs to be done. Many thanks.

david-leifker · 2024-09-05T14:12:40Z

@vignesh-hbk - Please take a look at the CI test failures. The tests can be run with ./gradlew locally with the following arguments. Linked to the failure logs for reference if you can view them.

:entity-registry:test
:metadata-io:test

vignesh-hbk · 2024-09-10T06:06:37Z

Hi @david-leifker - We ran both these tests and they passed. Please see below screenshots. Please let me know if further actions or info is needed from me. Thanks.

david-leifker · 2024-09-10T15:03:23Z

Updating the base branch which will rerun the tests. If it passes we're good to move forward. If not, it likely means that your local branch is behind and doesn't account for upstream changes in the project.

vignesh-hbk · 2024-09-12T08:27:44Z

@david-leifker - addressed the failing test cases and tried triggering the tests. Could you please check ? Many thanks.

david-leifker · 2024-09-12T19:20:48Z

Updated with latest master and still seeing failures in metadata-io:test, try pulling the latest changes on this branch (after my merge) and running with ./gradlew :metadata-io:test --rerun-tasks

Per the failure log, these are the kinds of failures I am seeing:

metadata-io > nonsearch > com.linkedin.metadata.search.indexbuilder.MappingsBuilderTest > testMappingsBuilder FAILED
    java.lang.AssertionError: expected [22] but found [27]
        at org.testng.Assert.fail(Assert.java:111)
        at org.testng.Assert.failNotEquals(Assert.java:1578)
        at org.testng.Assert.assertEqualsImpl(Assert.java:150)
        at org.testng.Assert.assertEquals(Assert.java:132)
        at org.testng.Assert.assertEquals(Assert.java:1419)
        at org.testng.Assert.assertEquals(Assert.java:1383)
        at org.testng.Assert.assertEquals(Assert.java:1429)
        at com.linkedin.metadata.search.indexbuilder.MappingsBuilderTest.testMappingsBuilder(MappingsBuilderTest.java:34)

metadata-io > nonsearch > com.linkedin.metadata.search.utils.BrowsePathUtilsTest > testGetDefaultBrowsePath STANDARD_OUT
    [Test, A, B]

metadata-io > nonsearch > com.linkedin.metadata.search.query.request.SearchQueryBuilderTest > testGetStandardFields FAILED
    java.lang.AssertionError: expected [22] but found [27]
        at org.testng.Assert.fail(Assert.java:111)
        at org.testng.Assert.failNotEquals(Assert.java:1578)
        at org.testng.Assert.assertEqualsImpl(Assert.java:150)
        at org.testng.Assert.assertEquals(Assert.java:132)
        at org.testng.Assert.assertEquals(Assert.java:1419)
        at org.testng.Assert.assertEquals(Assert.java:1383)
        at org.testng.Assert.assertEquals(Assert.java:1429)
        at com.linkedin.metadata.search.query.request.SearchQueryBuilderTest.testGetStandardFields(SearchQueryBuilderTest.java:379)

metadata-io > nonsearch > com.linkedin.metadata.search.query.request.SearchQueryBuilderTest > testQueryBuilderFulltext FAILED
    java.lang.AssertionError: expected [9] but found [14]
        at org.testng.Assert.fail(Assert.java:111)
        at org.testng.Assert.failNotEquals(Assert.java:1578)
        at org.testng.Assert.assertEqualsImpl(Assert.java:150)
        at org.testng.Assert.assertEquals(Assert.java:132)
        at org.testng.Assert.assertEquals(Assert.java:1419)
        at org.testng.Assert.assertEquals(Assert.java:1383)
        at org.testng.Assert.assertEquals(Assert.java:1429)
        at com.linkedin.metadata.search.query.request.SearchQueryBuilderTest.testQueryBuilderFulltext(SearchQueryBuilderTest.java:114)

metadata-io > nonsearch > com.linkedin.metadata.search.query.request.SearchQueryBuilderTest > testQueryBuilderStructured FAILED
    java.lang.AssertionError: expected [22] but found [27]
        at org.testng.Assert.fail(Assert.java:111)
        at org.testng.Assert.failNotEquals(Assert.java:1578)
        at org.testng.Assert.assertEqualsImpl(Assert.java:150)
        at org.testng.Assert.assertEquals(Assert.java:132)
        at org.testng.Assert.assertEquals(Assert.java:1419)
        at org.testng.Assert.assertEquals(Assert.java:1383)
        at org.testng.Assert.assertEquals(Assert.java:1429)
        at com.linkedin.metadata.search.query.request.SearchQueryBuilderTest.testQueryBuilderStructured(SearchQueryBuilderTest.java:212)

…roject#11335)

vignesh-hbk · 2024-09-13T07:34:48Z

@david-leifker - Hi David - Took the latest changes post merge and addressed the failing changes. Please review. All of the tests seem to have passed here as well. Please review and let us know if anything further needs to be done on our end. Many thanks for you help and patience.

vignesh-hbk · 2024-09-17T09:30:03Z

@david-leifker - Could you please help review the change? Many thanks

github-actions bot added product PR or Issue related to the DataHub UI/UX community-contribution PR or Issue raised by member(s) of DataHub Community labels Aug 1, 2024

coderabbitai bot reviewed Aug 1, 2024

View reviewed changes

vercel bot deployed to Preview August 1, 2024 07:46 View deployment

udbhav-hbk reviewed Aug 2, 2024

View reviewed changes

...io/src/test/java/com/linkedin/metadata/search/transformer/SearchDocumentTransformerTest.java Outdated Show resolved Hide resolved

coderabbitai bot reviewed Aug 5, 2024

View reviewed changes

vercel bot had a problem deploying to Preview August 5, 2024 13:10 Failure

vercel bot deployed to Preview September 5, 2024 13:59 View deployment

david-leifker self-assigned this Sep 5, 2024

vercel bot deployed to Preview September 10, 2024 15:18 View deployment

vignesh-hbk and others added 4 commits September 12, 2024 13:55

fix: support for non-string types in object fields

c34d360

Update SearchDocumentTransformerTest.java

118ff73

fix: support for non-string types in object fields

343944b

fix: support for non-string types in object fields

fd1c861

vignesh-hbk force-pushed the support-nonstring-types branch from 40bd73a to fd1c861 Compare September 12, 2024 08:25

vercel bot deployed to Preview September 12, 2024 08:58 View deployment

Merge branch 'master' into support-nonstring-types

488b416

vercel bot had a problem deploying to Preview September 12, 2024 19:51 Failure

Fix: tests for nonstrings type

85fa21c

vercel bot deployed to Preview September 13, 2024 06:49 View deployment

feat(ingest): make default rest sink mode env-configurable (datahub-p…

2a1f7b8

…roject#11335)

yoonhyejin and others added 7 commits September 13, 2024 12:33

feat: add acryl stories (datahub-project#11351)

517f8e1

fix: add cloud form & fix css (datahub-project#11362)

c8c6f56

fix: support for non-string types in object fields

877625d

Update SearchDocumentTransformerTest.java

c25708d

fix: support for non-string types in object fields

e3bedb7

Merge branch 'master' into support-nonstring-types

5e9e36c

fix: support for non-string types in object fields

d400d81

vercel bot deployed to Preview September 13, 2024 08:03 View deployment

Merge branch 'master' into support-nonstring-types

9a2adbb

vercel bot deployed to Preview September 17, 2024 12:51 View deployment

david-leifker approved these changes Sep 17, 2024

View reviewed changes

david-leifker merged commit c2977d8 into datahub-project:master Sep 17, 2024
41 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: support for non-string types in object fields #11066

fix: support for non-string types in object fields #11066

vignesh-hbk commented Aug 1, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Aug 1, 2024 •

edited

Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

coderabbitai bot left a comment

Nbagga14 commented Aug 20, 2024

david-leifker commented Sep 3, 2024 •

edited

Loading

vignesh-hbk commented Sep 4, 2024

david-leifker commented Sep 5, 2024

vignesh-hbk commented Sep 10, 2024

david-leifker commented Sep 10, 2024

vignesh-hbk commented Sep 12, 2024

david-leifker commented Sep 12, 2024 •

edited

Loading

vignesh-hbk commented Sep 13, 2024

vignesh-hbk commented Sep 17, 2024

fix: support for non-string types in object fields #11066

fix: support for non-string types in object fields #11066

Conversation

vignesh-hbk commented Aug 1, 2024 • edited by coderabbitai bot Loading

Checklist

Summary by CodeRabbit

coderabbitai bot commented Aug 1, 2024 • edited Loading

Review skipped

Walkthrough

Changes

Poem

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

Nbagga14 commented Aug 20, 2024

david-leifker commented Sep 3, 2024 • edited Loading

vignesh-hbk commented Sep 4, 2024

david-leifker commented Sep 5, 2024

vignesh-hbk commented Sep 10, 2024

david-leifker commented Sep 10, 2024

vignesh-hbk commented Sep 12, 2024

david-leifker commented Sep 12, 2024 • edited Loading

vignesh-hbk commented Sep 13, 2024

vignesh-hbk commented Sep 17, 2024

vignesh-hbk commented Aug 1, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Aug 1, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

david-leifker commented Sep 3, 2024 •

edited

Loading

david-leifker commented Sep 12, 2024 •

edited

Loading