Skip to content

SAC-31234: Exlcude un-auth streams from catalog while discovery#34

Open
satyendra101 wants to merge 4 commits into
masterfrom
SAC-31234/exclude-un-auth-streams
Open

SAC-31234: Exlcude un-auth streams from catalog while discovery#34
satyendra101 wants to merge 4 commits into
masterfrom
SAC-31234/exclude-un-auth-streams

Conversation

@satyendra101

@satyendra101 satyendra101 commented Jun 17, 2026

Copy link
Copy Markdown

Description of change

  • Handle discovery time check of streams by passing client (optional) to discovery
  • Excluded streams which return 403 error code when running discovery due to permission error
  • Added unit tests to validate the workflow.

Manual QA steps

  • Discovery running
  • Unittest running
  • Integration tests running

Risks

Rollback steps

  • revert this branch

AI generated code

https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code

  • this PR has been written with the help of GitHub Copilot or another generative AI tool

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds discovery-time permission checks so streams that the provided credentials cannot read (HTTP 403) are excluded from the generated Singer catalog, improving UX by preventing unusable streams from appearing in discovery results.

Changes:

  • Extend discovery flow to accept an optional OutbrainClient and prune inaccessible streams (and dependent child streams) from the catalog.
  • Introduce stream base classes and implement a permission probe for campaign.
  • Update unit tests and bump version/changelog for the new behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tap_outbrain/discover.py Adds access-check and pruning logic during discovery when a client is provided.
tap_outbrain/streams.py Introduces BaseStream and implements Campaign.check_access() used by discovery filtering.
tap_outbrain/client.py Adds OutbrainForbiddenError, config support on the client, and raises on HTTP 403.
tap_outbrain/__init__.py Wires discover-mode CLI path to construct a client and pass it into discovery.
tests/unittests/test_discover.py Adds unit tests for access-check/pruning and updates discovery tests for new signature.
tests/unittests/test_init.py Updates do_discover unit test for the new client parameter.
tests/unittests/test_catalog.py Updates parent/root stream assertions to match new BaseStream.parent behavior.
setup.py Bumps package version to 1.2.0.
CHANGELOG.md Documents the new discovery filtering behavior (with a minor wording fix requested).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tap_outbrain/discover.py Outdated
Comment thread tests/unittests/test_discover.py
Comment thread tests/unittests/test_discover.py
Comment thread tap_outbrain/streams.py
Comment thread CHANGELOG.md Outdated
Comment thread tap_outbrain/client.py
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.

2 participants