feat(taps): Abstract streams #2888

edgarrmondragon · 2025-02-26T05:14:17Z

Summary by Sourcery

Add support for abstract streams, which are not included in the catalog.

📚 Documentation preview 📚: https://meltano-sdk--2888.org.readthedocs.build/en/2888/

sourcery-ai · 2025-02-26T05:14:21Z

Reviewer's Guide by Sourcery

This pull request introduces an __abstract__ flag to the Stream class, allowing developers to define abstract streams that do not generate catalog entries. The _singer_catalog method in Tap was updated to exclude streams with this flag set to True. The selected property was also updated to return True if the stream is abstract and has selected descendents.

Updated class diagram for the Stream class

classDiagram
  class Stream {
    +selected_by_default: bool
    +__abstract__: bool
    +__init__(tap: Tap, name: str, schema: dict, ...)
    +selected(): bool
  }
  note for Stream "Added __abstract__ flag and updated selected() method"

File-Level Changes

Change	Details	Files
Added an `__abstract__` flag to the `Stream` class to prevent catalog entry generation for abstract streams.	Added an `__abstract__` boolean flag to the `Stream` class. Modified the `_singer_catalog` method in `Tap` to exclude streams with `__abstract__` set to `True`. Modified the `selected` property to return `True` if the stream is abstract and has selected descendents.	`singer_sdk/streams/core.py` `singer_sdk/tap_base.py`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!
Generate a plan of action for an issue: Comment @sourcery-ai plan on
an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

codecov · 2025-02-26T05:16:44Z

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.40%. Comparing base (57a9bb4) to head (335404a).

Files with missing lines	Patch %	Lines
singer_sdk/streams/core.py	91.66%	0 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (91.66%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2888      +/-   ##
==========================================
+ Coverage   91.33%   91.40%   +0.06%     
==========================================
  Files          62       62              
  Lines        5207     5214       +7     
  Branches      675      677       +2     
==========================================
+ Hits         4756     4766      +10     
+ Misses        319      317       -2     
+ Partials      132      131       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codspeed-hq · 2025-02-26T05:17:29Z

CodSpeed Performance Report

Merging #2888 will not alter performance

_{Comparing edgarrmondragon/feat/abstract-streams (335404a) with main (57a9bb4)}

Summary

✅ 7 untouched benchmarks

ReubenFrankel · 2025-02-27T13:49:06Z

singer_sdk/streams/core.py

@@ -1166,7 +1175,7 @@ def _sync_records(  # noqa: C901
                        )
                        raise

-                    if selected:
+                    if not self.__abstract__ and selected:


Suggested change

if not self.__abstract__ and selected:

if not selected:

Personally, I would remove the redundant assignment

selected = self.selected

above for clarity. Then this becomes

Suggested change

if not self.__abstract__ and selected:

if not self.selected:

as below.

ReubenFrankel · 2025-02-27T13:49:40Z

singer_sdk/streams/core.py

@@ -1234,7 +1243,7 @@ def sync(self, context: types.Context | None = None) -> None:
            self._write_replication_key_signpost(context, signpost)

        # Send a SCHEMA message to the downstream target:
-        if self.selected:
+        if not self.__abstract__ and self.selected:


Suggested change

if not self.__abstract__ and self.selected:

if not self.selected:

ReubenFrankel · 2025-02-27T13:55:58Z

tests/core/test_streams.py

    class MyTap(tap_class):
        def discover_streams(self):
-            return [SelectedStream(self), UnselectedStream(self)]
+            return [SelectedStream(self), UnselectedStream(self), AbstractStream(self)]


Would it be possible to avoid this somehow? As in, auto-discover/instantiate any parent streams that are abstract?

That's a very good point. I'll try to do that.

ReubenFrankel · 2025-02-27T14:01:30Z

singer_sdk/tap_base.py

@@ -338,6 +338,7 @@ def _singer_catalog(self) -> Catalog:
        return Catalog(
            (stream.tap_stream_id, stream._singer_catalog_entry)  # noqa: SLF001
            for stream in self.streams.values()
+            if not stream.__abstract__


Advantages/disadvantages of filtering out abstract streams here vs in self.streams?

ReubenFrankel · 2025-02-27T15:23:25Z

tests/core/test_parent_child.py

+        def get_records(self, context):  # noqa: ARG002
+            yield {"id": 1}
+            yield {"id": 2}
+
+        def generate_child_contexts(self, record, context):  # noqa: ARG002
+            yield {"pid": record["id"]}


Wondering if there is a better way to support this... It would be good if this could be encapsulated into generate_child_contexts only, but that currently requires at least one record (hence get_records implementation).

feat(taps): Abstract streams

5fc6499

edgarrmondragon added 3 commits February 25, 2025 23:19

Do not require name or schema

73d99e7

Generate a dummy name

747fd6f

Test child stream selection

335404a

edgarrmondragon self-assigned this Feb 26, 2025

edgarrmondragon added the Type/Tap Singer taps label Feb 26, 2025

ReubenFrankel reviewed Feb 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(taps): Abstract streams #2888

feat(taps): Abstract streams #2888

edgarrmondragon commented Feb 26, 2025 •

edited

Loading

sourcery-ai bot commented Feb 26, 2025 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

codecov bot commented Feb 26, 2025 •

edited

Loading

codspeed-hq bot commented Feb 26, 2025 •

edited

Loading

ReubenFrankel Feb 27, 2025

ReubenFrankel Feb 27, 2025 •

edited

Loading

ReubenFrankel Feb 27, 2025

ReubenFrankel Feb 27, 2025

edgarrmondragon Feb 27, 2025

ReubenFrankel Feb 27, 2025

ReubenFrankel Feb 27, 2025

	if not self.__abstract__ and self.selected:
	if not self.selected:

feat(taps): Abstract streams #2888

Are you sure you want to change the base?

feat(taps): Abstract streams #2888

Conversation

edgarrmondragon commented Feb 26, 2025 • edited Loading

Related

Summary by Sourcery

sourcery-ai bot commented Feb 26, 2025 • edited Loading

Reviewer's Guide by Sourcery

Updated class diagram for the Stream class

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

codecov bot commented Feb 26, 2025 • edited Loading

Codecov Report

codspeed-hq bot commented Feb 26, 2025 • edited Loading

CodSpeed Performance Report

Merging #2888 will not alter performance

Summary

ReubenFrankel Feb 27, 2025

Choose a reason for hiding this comment

ReubenFrankel Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

ReubenFrankel Feb 27, 2025

Choose a reason for hiding this comment

ReubenFrankel Feb 27, 2025

Choose a reason for hiding this comment

edgarrmondragon Feb 27, 2025

Choose a reason for hiding this comment

ReubenFrankel Feb 27, 2025

Choose a reason for hiding this comment

ReubenFrankel Feb 27, 2025

Choose a reason for hiding this comment

edgarrmondragon commented Feb 26, 2025 •

edited

Loading

sourcery-ai bot commented Feb 26, 2025 •

edited

Loading

codecov bot commented Feb 26, 2025 •

edited

Loading

codspeed-hq bot commented Feb 26, 2025 •

edited

Loading

ReubenFrankel Feb 27, 2025 •

edited

Loading