fix(ingest/mssql): remove lower() method from sql_common get_db_name #10773

sleeperdeep · 2024-06-25T12:19:08Z

Closes #9857

Summary by CodeRabbit

Documentation
- Updated the updating-datahub.md document to reflect significant changes in the DataHub system, including URN migration and breaking changes across versions.
New Features
- Introduced new parameters in CLI commands and enhanced Tableau ingestion.
- Added new dataset entries for various dimensions and facts in the Vertica database.
Bug Fixes
- Addressed inconsistencies in entity URN values and database name capitalizations across multiple integrations.
Chores
- Updated metadata structure for improved consistency and clarity in naming conventions.

sleeperdeep · 2024-06-25T12:36:22Z

@mayurinehate @hsheth2
Sorry guys, I made mistake and discarded all commits from previous PR. I created new one.

… add description updating-datahub

mayurinehate

Mostly looks good. Minor change suggested in example in doc.

mayurinehate · 2024-06-26T09:13:06Z

docs/how/updating-datahub.md

+
+  Old `urn`, where `data_base_name` is `Some_Database`:
+  ```
+  - urn:li:dataset:(urn:li:dataPlatform:paltform,some_database.schema.entitie,ENV)


Suggested change

- urn:li:dataset:(urn:li:dataPlatform:paltform,some_database.schema.entitie,ENV)

- urn:li:dataJob:(urn:li:dataFlow:(mssql,demodata.Foo.stored_procedures,PROD),Proc.With.SpecialChar)

Let's change the example to be of dataJob as only dataJob and container URNs have changed and not that of dataset.

mayurinehate · 2024-06-26T09:13:32Z

docs/how/updating-datahub.md

+  ```
+  New `urn`, where `data_base_name` is `Some_Database`:
+  ```
+  - urn:li:dataset:(urn:li:dataPlatform:paltform,Some_Database.schema.entitie,ENV)


Suggested change

- urn:li:dataset:(urn:li:dataPlatform:paltform,Some_Database.schema.entitie,ENV)

- urn:li:dataJob:(urn:li:dataFlow:(mssql,DemoData.Foo.stored_procedures,PROD),Proc.With.SpecialChar)

… add description updating-datahub (with dataJob urns)

mayurinehate

LGTM

sleeperdeep · 2024-06-26T12:29:58Z

@mayurinehate Thanks.
@hsheth2 sorry for my mistake, can you, please, look at this PR one more time?

sleeperdeep · 2024-08-28T11:32:03Z

@hsheth2 @mayurinehate
Hi guys! Can we merge this changes or something is not ok with it?

coderabbitai · 2024-08-30T09:08:03Z

Walkthrough

The pull request introduces significant updates to the DataHub system, particularly affecting the SQLAlchemySource class. The .lower() method has been removed from the get_db_name function, which impacts how database names are managed, requiring a migration to preserve original casing in URNs. The documentation has been revised to reflect various breaking changes, including updates to Protobuf CLI, OpenAPI specifications, and configuration requirements for the Glue source. Additionally, multiple JSON files have been updated to ensure consistency in entity URNs and naming conventions across different database contexts.

Changes

File	Change Summary
docs/how/updating-datahub.md	Updated to reflect removal of `lower` method from `get_db_name`, breaking changes, and new configurations for DataHub.
metadata-ingestion/src/datahub/ingestion/source/sql/sql_common.py	Modified `get_db_name` method to remove `.lower()` call, affecting database name casing; updated comments and logging.
metadata-ingestion/tests/integration/oracle/golden_test_ingest_with_database.json metadata-ingestion/tests/integration/oracle/golden_test_ingest_with_out_database.json	Updated `entityUrn` values and casing for `database` and `name` fields in multiple containers and datasets.
metadata-ingestion/tests/integration/sql_server/golden_files/golden_mces_mssql_no_db_to_file.json metadata-ingestion/tests/integration/sql_server/golden_files/golden_mces_mssql_no_db_with_filter.json metadata-ingestion/tests/integration/sql_server/golden_files/golden_mces_mssql_to_file.json metadata-ingestion/tests/integration/sql_server/golden_files/golden_mces_mssql_with_lower_case_urn.json	Updated `entityUrn` values and database/schema names for consistency; modified aspects to reflect new naming conventions.
metadata-ingestion/tests/integration/vertica/vertica_mces_with_db_golden.json	Updated `entityUrn` values, modified database name casing, and added new properties for containers and datasets.

Assessment against linked issues

Objective	Addressed	Explanation
Remove `.lower()` from `get_db_name` to fix URN casing inconsistency (#9857)	✅
Ensure URNs retain original casing across all relevant classes (#9857)	✅

Possibly related PRs

fix(ingestion/tableau): restructure the tableau graphql datasource query #11230: The changes in the get_db_name method of the SQLAlchemySource class in the main PR are directly related to the modifications made in the same method within the sql_common.py file, where the .lower() method was removed, affecting how database names are handled.

Suggested reviewers

hsheth2

Poem

🐇 In the garden where data flows,
A rabbit hops where the metadata grows.
With URNs bright, in their true case,
We celebrate changes, a joyful embrace!
No more lowercase, let the names shine,
In DataHub's realm, all is now fine! 🌼

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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @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.

… add description updating-datahub

… add description updating-datahub (with dataJob urns)

sleeperdeep · 2024-08-30T12:06:38Z

@hsheth2 How to reopen this PR back? Branch is still exists.

…name_method

…_method

sleeperdeep · 2024-09-03T13:45:26Z

@hsheth2 @mayurinehate
is failure of 'documentation / gh-pages' known issue or I should fix some else with current PR?

hsheth2 · 2024-09-03T19:41:38Z

@sleeperdeep the gh-pages one looked unrelated, so I've merged this in.

maggiehays · 2024-10-10T21:16:03Z

@coderabbitai summary

fix(ingest/mssql): remove lower() method from sql_common get_db_name

fff3ec0

github-actions bot added ingestion PR or Issue related to the ingestion of metadata community-contribution PR or Issue raised by member(s) of DataHub Community labels Jun 25, 2024

vercel bot had a problem deploying to Preview June 25, 2024 12:23 Failure

fix(ingest/mssql): remove lower() method from sql_common get_db_name;…

ca8f3b1

… add description updating-datahub

vercel bot had a problem deploying to Preview June 25, 2024 12:59 Failure

fix(ingest/mssql): remove lower() method from sql_common get_db_name;…

07394d1

… add description updating-datahub

vercel bot deployed to Preview June 25, 2024 14:42 View deployment

mayurinehate reviewed Jun 26, 2024

View reviewed changes

fix(ingest/mssql): remove lower() method from sql_common get_db_name;…

a48d618

… add description updating-datahub (with dataJob urns)

vercel bot deployed to Preview June 26, 2024 10:31 View deployment

mayurinehate approved these changes Jun 26, 2024

View reviewed changes

hsheth2 approved these changes Jul 8, 2024

View reviewed changes

hsheth2 added the merge-pending-ci A PR that has passed review and should be merged once CI is green. label Jul 8, 2024

sleeperdeep closed this Aug 30, 2024

sleeperdeep force-pushed the feature/fix_sql_common_get_db_name_method branch from b4af3cc to aa4061a Compare August 30, 2024 09:05

vercel bot deployed to Preview August 30, 2024 09:22 View deployment

sleeperdeep added 4 commits August 30, 2024 14:52

fix(ingest/mssql): remove lower() method from sql_common get_db_name

1da9093

fix(ingest/mssql): remove lower() method from sql_common get_db_name;…

6ea7f80

… add description updating-datahub

fix(ingest/mssql): remove lower() method from sql_common get_db_name;…

369018f

… add description updating-datahub

fix(ingest/mssql): remove lower() method from sql_common get_db_name;…

3a9550b

… add description updating-datahub (with dataJob urns)

sleeperdeep added 2 commits September 2, 2024 13:38

Merge branch 'lowercase_detached' into feature/fix_sql_common_get_db_…

4a23a7d

…name_method

Merge branch 'lower_detached' into feature/fix_sql_common_get_db_name…

574e980

…_method

sleeperdeep reopened this Sep 2, 2024

github-actions bot added docs Issues and Improvements to docs product PR or Issue related to the DataHub UI/UX devops PR or Issue related to DataHub backend & deployment labels Sep 2, 2024

vercel bot deployed to Preview September 2, 2024 11:26 View deployment

sleeperdeep and others added 2 commits September 3, 2024 12:58

Merge branch 'master' into feature/fix_sql_common_get_db_name_method

90d5d4c

revert to prev state

09aea86

vercel bot deployed to Preview September 3, 2024 10:32 View deployment

fix issue with vertica golden file

e3e324f

vercel bot deployed to Preview September 3, 2024 11:34 View deployment

fix issue with golden files

bd3d14d

vercel bot deployed to Preview September 3, 2024 12:14 View deployment

return path to prev state

4c423f7

vercel bot deployed to Preview September 3, 2024 12:44 View deployment

hsheth2 merged commit e34a628 into datahub-project:master Sep 3, 2024
67 of 68 checks passed

hsheth2 mentioned this pull request Sep 12, 2024

Feature/update logic mssql lineage #10959

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ingest/mssql): remove lower() method from sql_common get_db_name #10773

fix(ingest/mssql): remove lower() method from sql_common get_db_name #10773

sleeperdeep commented Jun 25, 2024 •

edited by coderabbitai bot

Loading

sleeperdeep commented Jun 25, 2024

mayurinehate left a comment

mayurinehate Jun 26, 2024

mayurinehate Jun 26, 2024

sleeperdeep Jun 26, 2024

mayurinehate Jun 26, 2024

sleeperdeep Jun 26, 2024

mayurinehate left a comment

sleeperdeep commented Jun 26, 2024

sleeperdeep commented Aug 28, 2024

coderabbitai bot commented Aug 30, 2024 •

edited

Loading

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

sleeperdeep commented Aug 30, 2024

sleeperdeep commented Sep 3, 2024

hsheth2 commented Sep 3, 2024

maggiehays commented Oct 10, 2024

	- urn:li:dataset:(urn:li:dataPlatform:paltform,some_database.schema.entitie,ENV)
	- urn:li:dataJob:(urn:li:dataFlow:(mssql,demodata.Foo.stored_procedures,PROD),Proc.With.SpecialChar)

fix(ingest/mssql): remove lower() method from sql_common get_db_name #10773

fix(ingest/mssql): remove lower() method from sql_common get_db_name #10773

Conversation

sleeperdeep commented Jun 25, 2024 • edited by coderabbitai bot Loading

Summary by CodeRabbit

sleeperdeep commented Jun 25, 2024

mayurinehate left a comment

Choose a reason for hiding this comment

mayurinehate Jun 26, 2024

Choose a reason for hiding this comment

mayurinehate Jun 26, 2024

Choose a reason for hiding this comment

sleeperdeep Jun 26, 2024

Choose a reason for hiding this comment

mayurinehate Jun 26, 2024

Choose a reason for hiding this comment

sleeperdeep Jun 26, 2024

Choose a reason for hiding this comment

mayurinehate left a comment

Choose a reason for hiding this comment

sleeperdeep commented Jun 26, 2024

sleeperdeep commented Aug 28, 2024

coderabbitai bot commented Aug 30, 2024 • edited Loading

Walkthrough

Changes

Assessment against linked issues

Possibly related PRs

Suggested reviewers

Poem

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

sleeperdeep commented Aug 30, 2024

sleeperdeep commented Sep 3, 2024

hsheth2 commented Sep 3, 2024

maggiehays commented Oct 10, 2024

sleeperdeep commented Jun 25, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Aug 30, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)