Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Stream sync context is now available to all instances methods as a Stream.context attribute #2529

Conversation

edgarrmondragon
Copy link
Collaborator

@edgarrmondragon edgarrmondragon commented Jul 10, 2024

Some context on the design here:

  • Adding a context parameter to RESTStream.get_new_paginator is a breaking change. It could be handled by catching a TypeError and retrying the call without the parameter, but this feels unnecessarily complicated.
  • I opted for adding a new context attribute to the Stream instance. It's only set when the sync starts and is wrapped by MappingProxyType to discourage mutating the context object.
  • The above makes the context parameter accepted by some methods a bit redundant, but I'm unsure whether it's worth deprecating and eventually removing it.
  • Docs: https://meltano-sdk--2529.org.readthedocs.build/en/2529/classes/singer_sdk.Stream.html#singer_sdk.Stream

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

Copy link

codspeed-hq bot commented Jul 10, 2024

CodSpeed Performance Report

Merging #2529 will not alter performance

Comparing 1520-feat-support-passing-context-to-the-pagination-class-initialization-scope (a56bc60) with main (fba8a44)

Summary

✅ 6 untouched benchmarks

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.17%. Comparing base (6256fe5) to head (a56bc60).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2529      +/-   ##
==========================================
- Coverage   89.18%   89.17%   -0.02%     
==========================================
  Files          54       54              
  Lines        4789     4784       -5     
  Branches      937      936       -1     
==========================================
- Hits         4271     4266       -5     
  Misses        361      361              
  Partials      157      157              

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

@edgarrmondragon edgarrmondragon changed the title feat: RESTStream.get_new_paginator is now passed the stream sync context feat: Stream sync context is now available to all instances methods as a Stream.context attribute Jul 10, 2024
@edgarrmondragon edgarrmondragon force-pushed the 1520-feat-support-passing-context-to-the-pagination-class-initialization-scope branch from c354cf7 to b1db975 Compare July 10, 2024 19:17
@edgarrmondragon edgarrmondragon force-pushed the 1520-feat-support-passing-context-to-the-pagination-class-initialization-scope branch 11 times, most recently from 6b7d2d2 to 500265e Compare July 10, 2024 22:06
@edgarrmondragon edgarrmondragon requested a review from a team as a code owner July 10, 2024 22:06
@edgarrmondragon edgarrmondragon force-pushed the 1520-feat-support-passing-context-to-the-pagination-class-initialization-scope branch 2 times, most recently from 206fa2b to a3c185d Compare July 10, 2024 22:43
@edgarrmondragon edgarrmondragon force-pushed the 1520-feat-support-passing-context-to-the-pagination-class-initialization-scope branch from a3c185d to a56bc60 Compare July 10, 2024 22:50
@edgarrmondragon edgarrmondragon merged commit 5500e45 into main Jul 10, 2024
35 checks passed
@edgarrmondragon edgarrmondragon deleted the 1520-feat-support-passing-context-to-the-pagination-class-initialization-scope branch July 10, 2024 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Support passing context to the pagination class initialization scope
1 participant