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.

Sign up for GitHub

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

Jump to bottom

feat(backend): structured properties and forms #9626

Merged
merged 26 commits into from
Jan 22, 2024
Merged

feat(backend): structured properties and forms #9626

merged 26 commits into from
Jan 22, 2024

Conversation

david-leifker
Copy link
Collaborator

@david-leifker david-leifker commented Jan 12, 2024 •
edited
Loading

Large set of backend changes that drive 2 major features: structured properties & forms. During the integration between these feature branches it was evident that a few areas of tech debt needed to be addressed. Additionally a few items will need follow-ups to limit scope here.

Structured Properties:

  • Creation & backwards compatible mutation
  • OpenAPI aspects
  • GraphQL
  • Validators (definition and instance)

Forms:

  • GraphQL

Misc Features:

  • RollbackService - Extracted from restli specific implementation to top level service
  • Entity Client - Remove need for application code to reason about restli vs java variants
  • Entity Client - Remove dependency of java entity client on restli entity client
  • EntityService - Remove dependency on restli entity client
  • Patch java package name refactor
  • Patch error messaging improvements
  • GraphQL: Generic entities & aspects

Follow-ups:

  • Backend: Structured properties delete
  • OpenAPI: Structured properties definition entities
  • OpenAPI: Forms support

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

@david-leifker
feat(entity-registry): entity registry plugins
1fc6766 
Add support for various custom plugins:
* validation
* RecordTemplate mutators
* MCL Side Effects
* MCE Side Effects
@david-leifker
Merge remote-tracking branch 'origin/master' into feat-entity-registr…
4835771 
…y-plugins
@david-leifker
wip
dda3d8e 
refactor to better support merging
started documentation in models-custom
@david-leifker
added examples
bb05e3e 
updated documentation
refactor to reduce generics use
added support for config endpoint
@david-leifker
Add subsequent disable ability
8569d67
@david-leifker
Merge remote-tracking branch 'origin/master' into feat-structured-pro…
2cb1460 
…perties
@david-leifker
feat(backend): structured properties and forms
43ece78 
* structured properties data model
* forms service
* rollback service
* entity client refactor
* configuration: logo & icon, base url
@github-actions github-actions bot added ingestion PR or Issue related to the ingestion of metadata product PR or Issue related to the DataHub UI/UX devops PR or Issue related to DataHub backend & deployment smoke_test Contains changes related to smoke tests labels Jan 12, 2024
@david-leifker
fix ingestion golden files
f266148 
update protobuf check
rename entity client factories correctly
@david-leifker
py fixes
528fa47 
java lint
@david-leifker @RyanHolstien
Update datahub-graphql-core/src/main/resources/entity.graphql
c12c84b 
Co-authored-by: RyanHolstien <RyanHolstien@users.noreply.github.com>
@david-leifker
Copy link
Collaborator Author

david-leifker commented Jan 17, 2024 •
edited
Loading

Missing allowedValues validation for structured properties when being written to an entity. This allows for unintended values to be written.

@david-leifker
fix validator for allowedValues
ce6af8c 
fix telemetry test
fix forms type for timestamps
chriscollins3456
chriscollins3456 reviewed Jan 17, 2024
View reviewed changes
Copy link
Collaborator

@chriscollins3456 chriscollins3456 left a comment

Choose a reason for hiding this comment

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

found one or two missing things and then some bulk stuff that we don't want to go to OSS

datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java Show resolved Hide resolved
...src/main/java/com/linkedin/datahub/graphql/resolvers/form/BatchSubmitFormPromptResolver.java Outdated Show resolved Hide resolved
datahub-graphql-core/src/main/resources/forms.graphql Outdated Show resolved Hide resolved
...-core/src/main/java/com/linkedin/datahub/graphql/resolvers/form/BatchVerifyFormResolver.java Outdated Show resolved Hide resolved
...main/java/com/linkedin/datahub/graphql/resolvers/search/SearchForEntitiesByFormResolver.java Outdated Show resolved Hide resolved
datahub-graphql-core/src/main/resources/search.graphql Outdated Show resolved Hide resolved
datahub-graphql-core/src/main/resources/search.graphql Outdated Show resolved Hide resolved
datahub-graphql-core/src/main/resources/search.graphql Outdated Show resolved Hide resolved
datahub-graphql-core/src/main/resources/search.graphql Outdated Show resolved Hide resolved
datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java Outdated Show resolved Hide resolved
chriscollins3456
chriscollins3456 approved these changes Jan 18, 2024
View reviewed changes
Copy link
Collaborator

@chriscollins3456 chriscollins3456 left a comment

Choose a reason for hiding this comment

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

things are looking good to me from here 👍

@david-leifker david-leifker merged commit 943bb57 into master Jan 22, 2024
62 checks passed
@david-leifker david-leifker deleted the feat-structured-properties branch January 22, 2024 17:46
This was referenced Jan 23, 2024
Merged
Merged
@Masterchen09 Masterchen09 mentioned this pull request Mar 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chriscollins3456 chriscollins3456 chriscollins3456 approved these changes

@RyanHolstien RyanHolstien RyanHolstien approved these changes

@jjoyce0510 jjoyce0510 Awaiting requested review from jjoyce0510

@shirshanka shirshanka Awaiting requested review from shirshanka

Assignees
No one assigned
Labels
devops PR or Issue related to DataHub backend & deployment ingestion PR or Issue related to the ingestion of metadata product PR or Issue related to the DataHub UI/UX smoke_test Contains changes related to smoke tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@david-leifker @chriscollins3456 @RyanHolstien