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(notebook): add data models for Notebook entity #4223

Merged
merged 6 commits into from
Mar 21, 2022
Merged

feat(notebook): add data models for Notebook entity #4223

merged 6 commits into from
Mar 21, 2022

Conversation

tc350981
Copy link
Contributor

@tc350981 tc350981 commented Feb 23, 2022 •
edited
Loading

This PR is to add pegasus schema to model Notebook data entity. RFC: #4237

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)

@github-actions
Copy link

github-actions bot commented Feb 23, 2022 •
edited
Loading

Unit Test Results (metadata ingestion)

       5 files         5 suites   49m 54s ⏱️
   365 tests    365 ✔️   0 💤 0 ❌
1 672 runs  1 634 ✔️ 38 💤 0 ❌

Results for commit 100b9ae.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Feb 23, 2022 •
edited
Loading

Unit Test Results (build & test)

  92 files  +17    92 suites  +17   19m 3s ⏱️ - 2m 8s
674 tests +49  615 ✔️ +50  59 💤 ±0  0 ❌  - 1 

Results for commit 100b9ae. ± Comparison against base commit 2d82531.

♻️ This comment has been updated with latest results.

@tc350981 tc350981 changed the title add data models for DataDoc entity add data models for Notebook entity Mar 15, 2022
@tc350981 tc350981 marked this pull request as ready for review March 15, 2022 17:45
@tc350981 tc350981 changed the title add data models for Notebook entity feat(notebook): add data models for Notebook entity Mar 15, 2022
@dexter-mh-lee
Copy link
Contributor

While making the above change, could you try merging latest master to see if the build issue gets resolved? Seems irrelevant to this change.

Xu Wang added 3 commits March 16, 2022 20:43
dexter-mh-lee
dexter-mh-lee reviewed Mar 17, 2022
View reviewed changes
Copy link
Contributor

@dexter-mh-lee dexter-mh-lee left a comment

Choose a reason for hiding this comment

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

Have a few more comments, but will put it on the RFC so we can discuss there. Once that discussion is finished, we can push this PR in.

metadata-models/src/main/pegasus/com/linkedin/notebook/CommonCellAttributes.pdl Outdated Show resolved Hide resolved
dexter-mh-lee
dexter-mh-lee reviewed Mar 18, 2022
View reviewed changes
Copy link
Contributor

@dexter-mh-lee dexter-mh-lee left a comment

Choose a reason for hiding this comment

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

@tc350981 I think the code looks good!

We want to make sure we mark these as IN BETA as we iterate in the beginning. Can you add this to all the new aspects (no need for internal objects) and add a doc field in the entity-registry.yml to specify this?

@tc350981
Copy link
Contributor Author

@tc350981 I think the code looks good!

We want to make sure we mark these as IN BETA as we iterate in the beginning. Can you add this to all the new aspects (no need for internal objects) and add a doc field in the entity-registry.yml to specify this?

Do you mean add a commend in all the aspect schema file and the data entity in entity-registry.yml to indicate this entity is in beta?

@dexter-mh-lee
Copy link
Contributor

Yes! So aspects and entity-registry.yaml. We want to start making this clear going forward so we can iterate faster on these new entities while not increasing our support load.

dexter-mh-lee
dexter-mh-lee approved these changes Mar 21, 2022
View reviewed changes
Copy link
Contributor

@dexter-mh-lee dexter-mh-lee left a comment

Choose a reason for hiding this comment

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

LGTM!

@dexter-mh-lee dexter-mh-lee merged commit 277ed43 into datahub-project:master Mar 21, 2022
maggiehays pushed a commit to maggiehays/datahub that referenced this pull request Aug 1, 2022
@tc350981 @maggiehays
feat(notebook): add data models for Notebook entity (datahub-project#…
9fbf884 
…4223)

* add Notebook data model

* resolve PR comments

1. add glossaryTerms aspect
2. make text in TextCell not searchable

* add dataPlatformInstance aspect and make notebookTool non-searchable

* make data doc cell title optional

* mark Notebook related aspects and entity as in BETA version

* add comments in entry-registry.yml for notebook entity

Co-authored-by: Xu Wang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gabe-lyons gabe-lyons gabe-lyons left review comments

@dexter-mh-lee dexter-mh-lee dexter-mh-lee approved these changes

Assignees

@dexter-mh-lee dexter-mh-lee

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tc350981 @dexter-mh-lee @gabe-lyons