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

enable relation between glossary term and datasets searchable #2806

Merged
merged 6 commits into from
Jul 22, 2021

Conversation

saxo-lalrishav
Copy link
Contributor

@saxo-lalrishav saxo-lalrishav commented Jun 30, 2021

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)

@saxo-lalrishav
Copy link
Contributor Author

#2805

@@ -7,5 +7,10 @@ record GlossaryTermAssociation {
/**
* Urn of the applied glossary term
*/
@Searchable = {
"fieldName": "glossaryTerms",
"fieldType": "TEXT_PARTIAL",
Copy link
Contributor

Choose a reason for hiding this comment

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

lets make the fieldType URN_PARTIAL- it parses urns more correctly than TEXT_PARTIAL does

Copy link
Contributor Author

@saxo-lalrishav saxo-lalrishav Jul 5, 2021

Choose a reason for hiding this comment

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

@lalrishav it looks like a comment you added may have been deleted- are you still planning on using TEXT_PARTIAL instead of TEXT_URN as a temporary fix for partial term searches?

yes can we do that pls ? as a temporary fix .

@Searchable = {
"fieldName": "glossaryTerms",
"fieldType": "TEXT_PARTIAL",
"hasValuesFieldName": "hasTerms"
Copy link
Contributor

Choose a reason for hiding this comment

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

do you expect to use the hasTerms filter? This filters datasets by those that have terms vs those that don't. How would you imagine this query being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no we dont need hasTerms, will remove it

@gabe-lyons
Copy link
Contributor

@lalrishav it looks like a comment you added may have been deleted- are you still planning on using TEXT_PARTIAL instead of TEXT_URN as a temporary fix for partial term searches?

@gabe-lyons
Copy link
Contributor

@saxo-lalrishav looks like you need to re-run gradle build and commit the newly generated avro schemas

@gabe-lyons
Copy link
Contributor

@saxo-lalrishav bump- you need to re-run gradle build and commit the newly generated avro schemas for CI to pass

@shirshanka
Copy link
Contributor

Screen Shot 2021-07-16 at 12 55 24 PM

relevant failure

Copy link
Contributor

@shirshanka shirshanka left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants