fix(ingestion/transformer): tranformer to replace the externalUrl in container properties

[sagar-salvi-apptware]

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). If a new feature has been added a Usage Guide has been added for the same.
• For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

Summary by CodeRabbit

• New Features

  • Introduced enhanced documentation for the "Replace ExternalUrl" functionality, with clearer distinctions between dataset and container transformations.
  • Added a new transformer type for handling external URLs within container properties.
  • Implemented new classes and methods to improve handling of external URLs in dataset and container contexts.

• Bug Fixes

  • Updated existing tests to reflect changes in transformer functionality, ensuring accuracy and reliability.

• Documentation

  • Enhanced documentation with detailed configuration options and examples for new transformers.

[coderabbitai]

Walkthrough

The recent changes enhance the metadata-ingestion module by refining how external URLs are handled in both dataset and container properties. New transformer classes, ReplaceExternalUrlDataset and ReplaceExternalUrlContainer, provide specialized functionality for URL replacements. The documentation has been clarified, and additional tests ensure robust performance for both datasets and containers.

Changes

Files	Change Summary
metadata-ingestion/docs/transformer/...	Renamed "Replace ExternalUrl" to "Replace ExternalUrl Dataset" and added "Replace ExternalUrl Container" with detailed configuration options.
metadata-ingestion/setup.py	Renamed ReplaceExternalUrl to ReplaceExternalUrlDataset and added a mapping for ReplaceExternalUrlContainer.
metadata-ingestion/src/datahub/.../dataset_transformer.py	Introduced ContainerTransformer and ContainerPropertiesTransformer classes for targeted transformations related to containers.
metadata-ingestion/src/datahub/.../replace_external_url.py	Replaced ReplaceExternalUrl with ReplaceExternalUrlDataset and added ReplaceExternalUrlContainer for handling external URLs in containers.
metadata-ingestion/tests/unit/.../test_transform_dataset.py	Added functions for container transformations and updated test cases to reflect new class names and functionalities.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant DatasetTransformer
    participant ContainerTransformer
    participant ReplaceUrl

    User->>DatasetTransformer: Initiate transformation
    DatasetTransformer->>ReplaceUrl: Call replace_url method
    ReplaceUrl-->>DatasetTransformer: Return transformed dataset
    User->>ContainerTransformer: Initiate container transformation
    ContainerTransformer->>ReplaceUrl: Call replace_url method
    ReplaceUrl-->>ContainerTransformer: Return transformed container

Loading

🐰 In the meadow where bunnies play,
New changes hop in, hip-hip-hooray!
URL replacements, tidy and neat,
With containers now, our tasks are complete.
So let's munch on greens and dance with delight,
For the code is now better, oh what a sight! 🌟

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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>.
  • 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 generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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.

[eboneil]

General feedback: instead of making this a new transformer, or making this a configuration of the transformer, can we just automatically detect it within the existing transformer? So like we are doing with datasets today, if there are containers, just go and make the replacement?

[sagar-salvi-apptware]

General feedback: instead of making this a new transformer, or making this a configuration of the transformer, can we just automatically detect it within the existing transformer? So like we are doing with datasets today, if there are containers, just go and make the replacement?

Problem with the current structure of transformer we cant have 2 different aspect, we can have multiple entity types but of same aspects.

[coderabbitai]

Actionable comments posted: 2

Outside diff range, codebase verification and nitpick comments (1)

metadata-ingestion/docs/transformer/dataset_transformer.md (1)

974-990: Consider clarifying the scope of the "Replace ExternalUrl Container" transformer.

To avoid any confusion, explicitly mention that this transformer is for replacing the externalUrl in container properties.

- Matches the full/partial string in the externalUrl of the container properties and replace that with the replacement string
+ Matches the full/partial string in the externalUrl of the container properties and replaces that with the replacement string.
+ This transformer is specifically for handling external URLs within container properties.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between bc75f7a and 444f06a.

Files selected for processing (5)

• metadata-ingestion/docs/transformer/dataset_transformer.md (2 hunks)
• metadata-ingestion/setup.py (1 hunks)
• metadata-ingestion/src/datahub/ingestion/transformer/dataset_transformer.py (2 hunks)
• metadata-ingestion/src/datahub/ingestion/transformer/replace_external_url.py (3 hunks)
• metadata-ingestion/tests/unit/test_transform_dataset.py (8 hunks)

Additional context used

Ruff

metadata-ingestion/tests/unit/test_transform_dataset.py

144-144: Do not perform function call models.StatusClass in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

Additional comments not posted (15)

metadata-ingestion/src/datahub/ingestion/transformer/replace_external_url.py (6)

23-26: Ensure regular expression pattern validity.

The replace_url method compiles the given pattern and uses it for replacement. Ensure that the input pattern is valid and does not lead to performance issues or ReDoS (Regular Expression Denial of Service).

---

Line range hint 29-45:
LGTM! Initialization of ReplaceExternalUrlDataset class.

The __init__ method correctly initializes the class with the provided configuration and context.

---

47-51: LGTM! Creation method for ReplaceExternalUrlDataset.

The create method correctly parses the configuration and returns an instance of the class.

---

Line range hint 53-75:
Ensure deep copy correctness in transform_aspect.

The transform_aspect method makes a deep copy of the input aspect before applying transformations. Ensure that this deep copy is necessary and does not lead to performance issues.

---

79-94: LGTM! Initialization of ReplaceExternalUrlContainer class.

The __init__ method correctly initializes the class with the provided configuration and context.

---

96-101: LGTM! Creation method for ReplaceExternalUrlContainer.

The create method correctly parses the configuration and returns an instance of the class.

metadata-ingestion/src/datahub/ingestion/transformer/dataset_transformer.py (3)

40-45: LGTM! Initialization of ContainerTransformer class.

The __init__ method correctly initializes the class by calling the superclass constructor.

---

46-47: LGTM! Entity types method for ContainerTransformer.

The entity_types method correctly returns the list of entity types handled by the transformer.

---

158-160: LGTM! Aspect name method for ContainerPropertiesTransformer.

The aspect_name method correctly returns the aspect name handled by the transformer.

metadata-ingestion/setup.py (1)

753-754: LGTM! Updated entry points for transformers.

The entry points for replace_external_url and replace_external_url_container are correctly updated to reflect the new class names.

metadata-ingestion/docs/transformer/dataset_transformer.md (1)

Line range hint 956-972: Documentation for "Replace ExternalUrl Dataset" looks good.

The new section provides clear configuration details and a useful example.

metadata-ingestion/tests/unit/test_transform_dataset.py (4)

1950-1982: LGTM!

The function run_container_transformer_pipeline is well-structured and follows the pattern of run_dataset_transformer_pipeline.

---

3925-3950: LGTM!

The test function test_replace_external_url_container_word_replace is comprehensive and ensures that the ReplaceExternalUrlContainer transformer works as expected.

---

3953-3978: LGTM!

The test function test_replace_external_regex_container_replace_1 is comprehensive and ensures that the ReplaceExternalUrlContainer transformer works as expected.

---

3981-4006: LGTM!

The test function test_replace_external_regex_container_replace_2 is comprehensive and ensures that the ReplaceExternalUrlContainer transformer works as expected.

[eboneil]

Thank you Sagar!

[coderabbitai]

Actionable comments posted: 3

Outside diff range, codebase verification and nitpick comments (3)

metadata-ingestion/src/datahub/ingestion/transformer/replace_external_url.py (2)

29-33: Typographical error in class docstring.

The docstring should use "replaces" instead of "replace".

"""Transformer that replaces the external URL for dataset properties."""

---

79-80: Typographical error in class docstring.

The docstring should use "replaces" instead of "replace".

"""Transformer that replaces the external URL for container properties."""

metadata-ingestion/docs/transformer/dataset_transformer.md (1)

982-982: Fix subject-verb agreement.

The verb should be "replaces" to agree with the subject "Transformer".

Matches the full/partial string in the externalUrl of the container properties and replaces that with the replacement string

Tools

LanguageTool

[uncategorized] ~982-~982: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...rnalUrl of the container properties and replace that with the replacement string ```ya...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 444f06a and d0b2556.

Files selected for processing (5)

• metadata-ingestion/docs/transformer/dataset_transformer.md (2 hunks)
• metadata-ingestion/setup.py (1 hunks)
• metadata-ingestion/src/datahub/ingestion/transformer/dataset_transformer.py (2 hunks)
• metadata-ingestion/src/datahub/ingestion/transformer/replace_external_url.py (3 hunks)
• metadata-ingestion/tests/unit/test_transform_dataset.py (8 hunks)

Files skipped from review as they are similar to previous changes (2)

• metadata-ingestion/setup.py
• metadata-ingestion/src/datahub/ingestion/transformer/dataset_transformer.py

Additional context used

LanguageTool

metadata-ingestion/docs/transformer/dataset_transformer.md

[uncategorized] ~982-~982: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...rnalUrl of the container properties and replace that with the replacement string ```ya...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)

Additional comments not posted (7)

metadata-ingestion/src/datahub/ingestion/transformer/replace_external_url.py (2)

70-73: Fix typographical errors in transform_aspect.

There are typographical errors in the variable name out_contianer_properties_aspect. It should be out_container_properties_aspect.

-            out_contianer_properties_aspect: ContainerPropertiesClass = copy.deepcopy(
+            out_container_properties_aspect: ContainerPropertiesClass = copy.deepcopy(
                in_container_properties_aspect
            )

-            out_contianer_properties_aspect.externalUrl = self.replace_url(
+            out_container_properties_aspect.externalUrl = self.replace_url(
                self.config.input_pattern,
                self.config.replacement,
                in_container_properties_aspect.externalUrl,
            )

-            return cast(Aspect, out_contianer_properties_aspect)
+            return cast(Aspect, out_container_properties_aspect)

---

120-123: Fix typographical errors in transform_aspect.

There are typographical errors in the variable name out_contianer_properties_aspect. It should be out_container_properties_aspect.

-            out_contianer_properties_aspect: ContainerPropertiesClass = copy.deepcopy(
+            out_container_properties_aspect: ContainerPropertiesClass = copy.deepcopy(
                in_container_properties_aspect
            )

-            out_contianer_properties_aspect.externalUrl = self.replace_url(
+            out_container_properties_aspect.externalUrl = self.replace_url(
                self.config.input_pattern,
                self.config.replacement,
                in_container_properties_aspect.externalUrl,
            )

-            return cast(Aspect, out_contianer_properties_aspect)
+            return cast(Aspect, out_container_properties_aspect)

metadata-ingestion/tests/unit/test_transform_dataset.py (5)

141-154: Ensure default aspect handling in make_generic_container_mcp.

The function correctly handles default values and constructs the MetadataChangeProposalWrapper. Ensure that the default aspect handling is consistent with other similar functions.

---

1952-1984: Ensure proper handling of both MCE and MCP inputs in run_container_transformer_pipeline.

The function correctly handles both MetadataChangeEventClass and MetadataChangeProposalWrapper inputs. Ensure that the transformer type and context management are consistent with other similar functions.

---

3927-3953: Ensure comprehensive test coverage in test_replace_external_url_container_word_replace.

The test function correctly validates the ReplaceExternalUrlContainer transformer. Ensure that the test covers various scenarios and edge cases, such as different URL patterns and replacement strings.

---

3955-3980: Ensure comprehensive test coverage in test_replace_external_regex_container_replace_1.

The test function correctly validates the ReplaceExternalUrlContainer transformer using regex patterns. Ensure that the test covers various regex scenarios and edge cases to ensure robustness.

---

3983-4008: Ensure comprehensive test coverage in test_replace_external_regex_container_replace_2.

The test function correctly validates the ReplaceExternalUrlContainer transformer using different regex patterns. Ensure that the test covers various regex scenarios and edge cases to ensure robustness.