sdk
sdk copied to clipboard
bug: `No records returned in stream` warning for ignored streams
Singer SDK Version
0.45.1
Is this a regression?
- [ ] Yes
Python Version
3.9
Bug scope
Taps (catalog, state, etc.)
Operating System
No response
Description
Ignoring no records for a stream surfaces another UserWarning I didn't previously spot after working on #2903, #2906 and #2907:
No records returned in stream
https://github.com/ReubenFrankel/tap-f1/actions/runs/13897554712
Should there a be a UserWarning emitted here if a developer is explicitly choosing to ignore no records for streams?
https://github.com/meltano/sdk/blob/c6672eb70002c7430f5db30fba05bb21cf9f0c11/singer_sdk/testing/tap_tests.py#L106:L111
Link to Slack/Linen
No response
Should there a be a
UserWarningemitted here if a developer is explicitly choosing to ignore no records for streams?
I would say probably not.
Is this warning independent from the error in the linked workflow?
FAILED tests/test_core.py::TestTapF1::test_tap_stream_primary_keys[driver_standings] - KeyError: 'position'
Is this warning independent from the error in the linked workflow?
Yes, I just linked the workflow where this first cropped up after I started ignoring no records for the sprint_results stream:
No records returned in stream 'sprints_results'.
I have since fixed the KeyError issue: https://github.com/ReubenFrankel/tap-f1/pull/111
You can see the same warning appears in recent successful runs as well: https://github.com/ReubenFrankel/tap-f1/actions/runs/14191574119
thanks @ReubenFrankel,
Does the approach in https://github.com/meltano/sdk/pull/2946 make sense then?
I like the idea of using pytest.skip instead of simply passing the test. Wdyt?
Does the approach in #2946 make sense then?
Looks good to me - I prefer that approach as well. 🙂
I started working on a fix too, which included a refactor of ignore_no_records as an instance property, if you want to include that in your PR as well: https://github.com/ReubenFrankel/sdk/commit/e76b99878cb5d3bd0c77296617cac0e200f504f8
included a refactor of
ignore_no_recordsas an instance property, if you want to include that in your PR as well: ReubenFrankel@e76b998
I'm seeing an error when I cherry pick that:
File "/Users/edgarramirez/Arch/sdk/singer_sdk/testing/templates.py", line 204, in <module>
class AttributeTestTemplate(StreamTestTemplate[TapTestRunner]):
TypeError: <class 'singer_sdk.testing.templates.StreamTestTemplate'> is not a generic class
but I'm happy to review a PR to make that change.
I'm seeing an error when I cherry pick that:
File "/Users/edgarramirez/Arch/sdk/singer_sdk/testing/templates.py", line 204, in <module> class AttributeTestTemplate(StreamTestTemplate[TapTestRunner]): TypeError: <class 'singer_sdk.testing.templates.StreamTestTemplate'> is not a generic class
I'll follow up with a working one once #2946 is merged then. 😅
https://github.com/ReubenFrankel/tap-f1/actions/runs/14206461897 No warnings! 😍 Thank you @edgarrmondragon