Skip to content

Conversation

EmmetAVS
Copy link
Contributor

Related to issue 13541.

Golden files for postgres integration have been updated to account for the change and the added support for stored procedures.

@github-actions 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 Jul 16, 2025
@datahub-cyborg datahub-cyborg bot added the needs-review Label for PRs that need review from a maintainer. label Jul 16, 2025
@sgomezvillamor sgomezvillamor self-requested a review July 18, 2025 07:59
Copy link
Contributor

@sgomezvillamor sgomezvillamor left a comment

Choose a reason for hiding this comment

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

Overall LGTM Thanks for the contrib
I left some minor and mostly cosmetic comments

default=True,
description="Include ingest of stored procedures. Requires access to the 'sys' schema.",
)
include_stored_procedures_code: bool = Field(
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this flag being used, if that's the case, let's remove it

@datahub-cyborg datahub-cyborg bot added pending-submitter-response Issue/request has been reviewed but requires a response from the submitter and removed needs-review Label for PRs that need review from a maintainer. labels Jul 18, 2025
EmmetAVS and others added 2 commits July 19, 2025 19:17
Co-authored-by: Sergio Gómez Villamor <sgomezvillamor@gmail.com>
Co-authored-by: Sergio Gómez Villamor <sgomezvillamor@gmail.com>
@datahub-cyborg datahub-cyborg bot added needs-review Label for PRs that need review from a maintainer. and removed pending-submitter-response Issue/request has been reviewed but requires a response from the submitter labels Jul 20, 2025
…ld and edited description of the include_stored_procedures field
@EmmetAVS EmmetAVS requested a review from sgomezvillamor July 20, 2025 02:49
@datahub-cyborg datahub-cyborg bot added pending-submitter-response Issue/request has been reviewed but requires a response from the submitter and removed needs-review Label for PRs that need review from a maintainer. labels Jul 21, 2025
Copy link
Contributor

@sgomezvillamor sgomezvillamor left a comment

Choose a reason for hiding this comment

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

LGTM

I just left a final code suggestion with a tiny fix for a string interpolation that is causing the tests to fail.

As soon as tests pass, this can be merged

Thanks for the contrib!

@datahub-cyborg datahub-cyborg bot added merge-pending-ci A PR that has passed review and should be merged once CI is green. and removed pending-submitter-response Issue/request has been reviewed but requires a response from the submitter labels Jul 21, 2025
Co-authored-by: Sergio Gómez Villamor <sgomezvillamor@gmail.com>
@EmmetAVS
Copy link
Contributor Author

LGTM

I just left a final code suggestion with a tiny fix for a string interpolation that is causing the tests to fail.

As soon as tests pass, this can be merged

Thanks for the contrib!

Yeah, no problem, thanks for the help/advice.

@acryl-hyejin acryl-hyejin merged commit 47e436f into datahub-project:master Jul 22, 2025
57 checks passed
kartikey-visa pushed a commit to kartikey-visa/datahub that referenced this pull request Jul 23, 2025
…ub-project#14102)

Co-authored-by: root <root@Aarush-PC.localdomain>
Co-authored-by: Sergio Gómez Villamor <sgomezvillamor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution PR or Issue raised by member(s) of DataHub Community ingestion PR or Issue related to the ingestion of metadata merge-pending-ci A PR that has passed review and should be merged once CI is green.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants