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

[ER Diagrams] Add ER diagram APIs and sample data #18021

Merged
merged 32 commits into from
Oct 28, 2024
Merged

[ER Diagrams] Add ER diagram APIs and sample data #18021

merged 32 commits into from
Oct 28, 2024

Conversation

OnkarVO7
Copy link
Contributor

@OnkarVO7 OnkarVO7 commented Sep 27, 2024

Describe your changes:

Add ER diagram APIs and sample data

UI :

  • Added TableDetailPage and SchemaDetailPage tab to class base to further extend them on Collate side
  • Added support to edit Column Constraints in the DisplayName edit Modal.
  • Added support to add and edit Table Constraints in TableDetail Page
Screen.Recording.2024-10-27.at.3.57.34.PM.mov

Type of change:

  • Bug fix
  • Improvement
  • [s] New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Copy link
Contributor

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

Copy link
Contributor

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

Copy link
Contributor

github-actions bot commented Sep 27, 2024

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 64%
64.11% (38016/59298) 40.67% (15147/37245) 42.75% (4541/10623)

if (depth <= 0) {
return;
}
es.org.elasticsearch.action.search.SearchRequest searchRequest =
Copy link
Collaborator

Choose a reason for hiding this comment

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

we need to put full package names here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The file is references a specific class for the SearchRequest. hence the full name here. It is done for other APIs as well

String direction,
boolean deleted)
throws IOException {
if (depth <= 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

how can be depth be below 0 here? we should throw an error not return

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a function that will be called recursively. The depth is there for handling that
E.g by default depth with start at 3 and then with each recursion it'll get decremented unless it reaches 0

if (depth <= 0) {
return;
}
os.org.opensearch.action.search.SearchRequest searchRequest =
Copy link
Collaborator

Choose a reason for hiding this comment

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

avoid full package names like this here

Copy link

Copy link

1 similar comment
Copy link

@@ -644,6 +645,41 @@ public DatabaseSchema deleteDataProfilerConfig(
return addHref(uriInfo, databaseSchema);
}

@GET
@Path("/getEntityRelationship")
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets not add "Get" , lets just call it as /entityRelationship. GET is indicated by the HTTP Method

Copy link
Collaborator

Choose a reason for hiding this comment

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

@OnkarVO7 any reason not to add this at database level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harshach Main to add this at schema level was to make sure the performance remains consistent. We can extend database level too once this is out

@harshach
Copy link
Collaborator

@OnkarVO7 lets make sure we are adding EDIT_ENTITY_RELATIONSHIP similar to EDIT_LINEAGE.
@Ashish8689 this needs to be addressed in the UI as well.
We give permissions to EDIT_ALL or EDIT_ENTITY_RELATIONSHIP

harshach
harshach previously approved these changes Oct 28, 2024
Copy link

sonarcloud bot commented Oct 28, 2024

Copy link

sonarcloud bot commented Oct 28, 2024

@OnkarVO7 OnkarVO7 merged commit 4a0c840 into main Oct 28, 2024
27 checks passed
@OnkarVO7 OnkarVO7 deleted the er_diagrams branch October 28, 2024 14:56
harshach pushed a commit that referenced this pull request Nov 3, 2024
* Add ER diag APIs and sample data

* fix pylint

* formatting fixes2

* fixed es client return

* fixed os client return

* supported TableDetailPage tabs as classBase for supporting collate only tabs

* Added schema Apis

* change the base class to .ts and move the component in the util files

* beautify function arguments

* Added optimizations

* Ingestion changes

* svg dimension change

* supported class base tab in databaseSchema

* supported classBase action button in schema table name column

* added further keys data for constraint modal

* fix sonar issue

* remove old method to override edit action on column and shifted to DisplayNameModal for fields

* supported table right panel component to further extends on collate side

* minor fix around duplicate constraint

* added support to update table constraints and column constraints in the UI

* code optimization and minor fixes

* review comments and multi col fix

* added queryFilter option in NodeSuggestion and tableConstrainst to fetch and use only in service tables

---------

Co-authored-by: Ashish Gupta <ashish@getcollate.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ingestion safe to test Add this label to run secure Github workflows on PRs UI UI specific issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants