added unauth changes - #130
Open
mukeshbhatt18gl wants to merge 12 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces handling for 403/unauthorized Square API responses during discovery so that streams without read access are excluded from the produced Singer catalog, and adds unit tests around the new access-check behavior.
Changes:
- Add
SquareForbiddenErrorand raise it on 403 responses from Square API calls. - Add per-stream access probing (
check_access/_probe_access) and apply it during discovery to remove forbidden streams from the catalog. - Add unit tests for discovery/access behavior and bump package/dependency versions.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unittests/test_discovery.py | Adds unit/integration tests for discovery filtering and stream access probes. |
| tap_square/streams.py | Adds access probing helpers and check_access() implementations used during discovery. |
| tap_square/discover.py | Applies access checks during discovery and excludes forbidden streams from the generated catalog. |
| tap_square/client.py | Introduces SquareForbiddenError and raises it when Square returns HTTP 403. |
| tap_square/init.py | Updates CLI entrypoint to construct a client and pass it into discovery. |
| setup.py | Bumps tap version and dependency pins. |
| CHANGELOG.md | Documents the new release entry for the access-check/discovery behavior and dependency bumps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| window_start = window_end | ||
|
|
||
|
|
||
| LOGGER = singer.get_logger() |
Comment on lines
20
to
27
| client = SquareClient(args.config, args.config_path) | ||
|
|
||
| if args.discover: | ||
| catalog = discover(client, is_sandbox) | ||
| write_catalog(catalog) | ||
| else: | ||
| catalog = args.catalog if args.catalog else discover(client, is_sandbox) | ||
| sync(args.config, args.config_path, args.state, catalog) |
Comment on lines
+35
to
+55
| # Mark 'items' and 'categories' as inaccessible | ||
| def fake_check_access(stream_instance): | ||
| return stream_instance.tap_stream_id not in ('items', 'categories') | ||
|
|
||
| with patch.dict( | ||
| {name: type(cls.__name__, (cls,), {'check_access': lambda self: fake_check_access(self)}) | ||
| for name, cls in STREAMS.items()}, | ||
| ): | ||
| # Simpler approach: patch check_access on the stream objects | ||
| pass | ||
|
|
||
| # Direct approach using side_effect on instantiated streams | ||
| originals = {} | ||
| for name, cls in STREAMS.items(): | ||
| orig = cls.check_access | ||
| originals[name] = orig | ||
| if name in ('items', 'categories'): | ||
| cls.check_access = lambda self: False | ||
| else: | ||
| cls.check_access = lambda self: True | ||
|
|
Comment on lines
+3
to
+6
| ## [v2.4.0] | ||
| * Exclude 403-forbidden streams from discovery [#130](https://github.com/singer-io/tap-square/pull/130) | ||
| * Bump dependencies for compliance | ||
|
|
This file contains hidden or 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
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.
Description of change
SAC-31240
Manual QA steps
Risks
Rollback steps
AI generated code
https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code