feat(ingest): add removing partition pattern in spark lineage #6605
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
in aws emr, when I use spark lineage, input table is s3 location. but It include partition location. so It doesn't match other ingestion (like glue ingestion)
ssilb4
changed the title
anshbansal
added
the
community-contribution
jjoyce0510
reviewed
Jan 9, 2023
| | spark.datahub.metadata.dataset.env | | PROD | [Supported values](https://datahubproject.io/docs/graphql/enums#fabrictype). In all other cases, will fallback to PROD | | ||
| | spark.datahub.metadata.table.hive_platform_alias | | hive | By default, datahub assigns Hive-like tables to the Hive platform. If you are using Glue as your Hive metastore, set this config flag to `glue` | | ||
| | spark.datahub.metadata.include_scheme | | true | Include scheme from the path URI (e.g. hdfs://, s3://) in the dataset URN. We recommend setting this value to false, it is set to true for backwards compatibility with previous versions | | ||
| | spark.datahub.metadata.remove_partition_pattern | | | Remove partition pattern. (e.g. /partition=\d+) It change database/table/partition=123 to database/table | |
There was a problem hiding this comment.
Please add the default value! What is the default?
There was a problem hiding this comment.
Also what are sample values?
There was a problem hiding this comment.
I don't think it need to have default value. If it is null, It doesn't change any value.
I thought description is sample value, should I do more? (value: /partition=\d+)
jjoyce0510
reviewed
Jan 9, 2023
|
|
||
| private static String getRemovePartitionPattern(Config datahubConfig) { | ||
| return datahubConfig.hasPath(REMOVE_PARTITION_PATTERN) ? datahubConfig.getString(REMOVE_PARTITION_PATTERN) | ||
| : ""; |
There was a problem hiding this comment.
Minor: Why not return null? Why empty string?
jjoyce0510
reviewed
Jan 9, 2023
| } else { | ||
| return uri.getHost() + uri.getPath(); | ||
| String uriPath = includeScheme ? uri.toString() : uri.getHost() + uri.getPath(); | ||
| if (!removePartitionPattern.equals("")) { |
There was a problem hiding this comment.
If we returned null in previous version we can check whether its not equal to null instead of empty string.
jjoyce0510
requested changes
Jan 9, 2023
empty string to null
because default value is null
jjoyce0510
approved these changes
Jan 9, 2023
|
Looks like build is failing. Going to update to the latest.. |
|
hmm.. should I fix more? I don't understand why smoke test failed. |
ericyomi
pushed a commit
to ericyomi/datahub
that referenced
this pull request
Feb 8, 2023
…atahub-project#6605) Co-authored-by: John Joyce <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
in aws emr, when I use spark lineage, input table is s3 location. but It include partition location. so It doesn't match other ingestion (like glue ingestion)
Remove partition pattern. (e.g. /partition=\d+) It change database/table/partition=123 to database/table
Checklist