Skip to content

Updated discovery stream access#140

Open
rsaha-qlik wants to merge 14 commits into
gl-masterfrom
SAC-30946-discovery-auth-perm
Open

Updated discovery stream access#140
rsaha-qlik wants to merge 14 commits into
gl-masterfrom
SAC-30946-discovery-auth-perm

Conversation

@rsaha-qlik

@rsaha-qlik rsaha-qlik commented Jun 9, 2026

Copy link
Copy Markdown

Description of change

Summary

Added discovery-time access checks to tap-mambu so that streams the API credentials cannot access are excluded from the catalog rather than causing a runtime error.

Changes

tap_mambu/helpers/discover.py

  • Added STREAM_PROBE_CONFIG — a static mapping of each stream to its probe parameters (method, path, version, apikey_type). Child streams (cards, loan_repayments) are marked with a parent key instead of a direct probe config.
  • Added check_stream_access(client, stream_name) -> bool:
    • Probes each stream using its actual sync method (GET or POST) with minimal params (pageSize=1)
    • Returns False on MambuUnauthorizedError, MambuForbiddenError, or MambuNoAuditApikeyInConfig
    • Returns True on any other MambuError (server responded → credentials are valid)
    • Re-raises non-Mambu errors (e.g. network failures)
  • Updated discover()discover(client):
    • Two-pass approach: top-level streams are probed first; child streams are included only if their parent stream is accessible
    • Logs a warning for each excluded stream
    • Raises an exception if no streams are accessible

tap_mambu/__init__.py

  • Updated do_discover() to accept and pass client to discover(client)

tests/unittests/test_discover.py (new file)

  • TestCheckStreamAccess: covers accessible (True), 401/403/missing-audit-key (False), non-auth MambuError fallback (True), GET vs POST method selection, audit apikey type, v1 version, pageSize=1 param, re-raise of non-Mambu errors
  • TestDiscover: all accessible, inaccessible stream excluded, child stream excluded/included based on parent accessibility, child streams never probed directly, warning logged for excluded streams, all-inaccessible raises exception, incremental replication key marked automatic in metadata

CHANGELOG.md

  • Added 4.7.0 entry

Testing

python -m pytest [test_discover.py](http://_vscodecontentref_/0) -v

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
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 discovery-time stream accessibility probing so streams that the provided credentials can’t access are excluded from the generated Singer catalog (instead of failing later during sync).

Changes:

  • Added per-stream probe configuration and check_stream_access() to test access during discovery.
  • Updated discover() to filter inaccessible streams (and exclude child streams when their parent is inaccessible).
  • Added unit tests for access probing + discovery filtering, and bumped version/changelog.

Reviewed changes

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

Show a summary per file
File Description
tap_mambu/helpers/discover.py Adds probe config + access-checking logic and updates discovery to exclude inaccessible streams.
tap_mambu/__init__.py Passes the instantiated client into discovery.
tests/unittests/test_discover.py New unit tests covering stream probing behavior and discovery filtering rules.
setup.py Bumps package version to 4.7.0.
CHANGELOG.md Adds 4.7.0 entry describing discovery-time access checks.

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

Comment thread tap_mambu/helpers/discover.py Outdated
Comment thread tap_mambu/helpers/discover.py Outdated
Comment thread tap_mambu/helpers/discover.py Outdated
Comment thread tap_mambu/helpers/discover.py
Comment thread tap_mambu/helpers/discover.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 9 out of 9 changed files in this pull request and generated 6 comments.

Comment thread tap_mambu/helpers/discover.py
Comment thread tap_mambu/helpers/discover.py
Comment thread tap_mambu/helpers/discover.py Outdated
Comment thread tap_mambu/helpers/discover.py Outdated
Comment thread tap_mambu/helpers/discover.py Outdated
Comment thread CHANGELOG.md 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 9 out of 9 changed files in this pull request and generated 3 comments.

Comment thread CHANGELOG.md Outdated
Comment thread tests/unittests/test_discover.py Outdated
Comment thread mambu_tests/conftest.py
rsaha-qlik and others added 8 commits June 9, 2026 13:58
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>
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