Skip to content

Added implementation to exclude unauthorized streams from discovery#57

Open
akkumar-qlik wants to merge 7 commits into
masterfrom
SAC-30930-un-auth-streams-should-exclude-from-catalog-while-discovery
Open

Added implementation to exclude unauthorized streams from discovery#57
akkumar-qlik wants to merge 7 commits into
masterfrom
SAC-30930-un-auth-streams-should-exclude-from-catalog-while-discovery

Conversation

@akkumar-qlik

Copy link
Copy Markdown
Contributor

Description of change

https://qlik-dev.atlassian.net/browse/SAC-30930
Excluded unauthorized streams from catalog during discovery
Child streams are automatically excluded when their parent stream is inaccessible.
Added unit test case and updated integration test accordingly

Manual QA steps

  • Discovery: Running
  • Sync: Running
  • Unit Tests: Running
  • Integration test: 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

@akkumar-qlik akkumar-qlik self-assigned this Jun 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a discovery-time permission probe to exclude streams that return HTTP 403 from the generated Singer catalog, and updates unit/integration tests and versioning to reflect the new behavior.

Changes:

  • Add PardotForbiddenError and raise it on HTTP 403 responses in the client.
  • Add discovery-time filtering: check per-stream access, prune child streams when their parent is inaccessible, and fail discovery only if no parent streams are accessible.
  • Add/adjust unit tests and update integration-test expectations; bump version and changelog.

Reviewed changes

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

Show a summary per file
File Description
tap_pardot/client.py Introduces PardotForbiddenError and raises it on HTTP 403 to support permission-aware behavior.
tap_pardot/discover.py Adds _apply_access_checks and _prune_inaccessible_children, invoked during discover() to exclude unauthorized streams.
tap_pardot/streams.py Adds Stream.check_access() to probe per-stream permissions during discovery.
tests/unittests/test_discover.py Adds unit tests for access-check behavior and stream permission probing.
tests/base.py Updates integration-test expected stream names to allow excluding forbidden streams via metadata.
setup.py Bumps package version to 2.1.0.
CHANGELOG.md Documents the new discovery behavior in 2.1.0.

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

Comment thread tap_pardot/streams.py
Comment thread tap_pardot/discover.py
Comment thread tests/unittests/test_discover.py
Comment thread tests/unittests/test_discover.py
Comment thread tests/unittests/test_discover.py Outdated
Comment thread tests/base.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread tap_pardot/streams.py
Comment thread CHANGELOG.md Outdated
akkumar-qlik and others added 4 commits June 5, 2026 03:00
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@RushiT0122 RushiT0122 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.

parent-tap-stream-id is missing from catalog which is handled in PR#54. Please merge those changes in this PR and retest the changes.

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.

3 participants