Skip to content

Added stream access check in discovery#16

Open
rsaha-qlik wants to merge 4 commits into
SAC-28771/python-upgradefrom
SAC-31239-discovery-auth-perm
Open

Added stream access check in discovery#16
rsaha-qlik wants to merge 4 commits into
SAC-28771/python-upgradefrom
SAC-31239-discovery-auth-perm

Conversation

@rsaha-qlik

Copy link
Copy Markdown

Description of change

https://qlik-dev.atlassian.net/browse/SAC-31239

Summary

This PR adds discovery-time stream access filtering to tap-saasoptics so inaccessible streams are excluded from the catalog, while preserving existing behavior when no client is provided.

What changed

  • Updated discovery to support discover(client=None):
    • Probes stream access via lightweight GET calls.
    • Excludes streams that return SaaSOpticsForbiddenError.
    • Logs excluded stream names.
    • Raises SaaSOpticsForbiddenError if no streams are accessible.
  • Wired discover mode to pass the authenticated client into discovery (do_discover(client)).
  • Added unit coverage for discovery access behavior:
    • Returns all streams without a client.
    • Excludes only forbidden streams with a client.
    • Fails when all streams are forbidden.
  • Updated release metadata:
    • Version bumped to 1.3.0.
    • Changelog entry added for stream exclusion behavior.

Why

Some SaaSOptics accounts have partial API permissions. This prevents discovery from failing entirely when only some streams are restricted, while still failing clearly when nothing is accessible.

Validation

Ran targeted unit tests:

  • ../env/Scripts/python.exe -m pytest tests/unittests/test_discover.py tests/unittests/test_client.py tests/unittests/test_sync.py

Result:

  • 39 passed (with existing deprecation warnings unrelated to this change)

Notes

  • Backward compatibility is preserved for callers that use discover() without a client.

Manual QA steps

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

This PR adds discovery-time stream access filtering so the tap’s catalog excludes streams that the authenticated token cannot access, while keeping the existing behavior when discovery is run without a client.

Changes:

  • Updated discover(client=None) to probe stream access and exclude streams that raise SaaSOpticsForbiddenError, failing discovery if none are accessible.
  • Wired CLI discovery to pass the authenticated SaaSOpticsClient into discovery.
  • Added unit tests for the new discovery access behavior and bumped release metadata (version + changelog).

Reviewed changes

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

Show a summary per file
File Description
tap_saasoptics/discover.py Adds optional client-based access checks and filtering during catalog generation.
tap_saasoptics/__init__.py Passes the authenticated client into discovery when running in --discover mode.
tests/unittests/test_discover.py Adds unit coverage for filtering/raising behavior when a client is provided.
setup.py Bumps package version to 1.3.0.
CHANGELOG.md Documents the new discovery stream exclusion behavior.

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

Comment thread tap_saasoptics/discover.py
Comment thread setup.py

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread CHANGELOG.md
# Changelog

## 1.3.0
* Exclude inaccessible streams during discovery and fail discovery when no streams are accessible. [#16] (http://github.com/singer-io/tap-saasoptics/pull/16)
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