Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: singer-io/tap-fixerio doesn't support --discover #1948

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

edgarrmondragon
Copy link
Collaborator

Copy link

netlify bot commented Feb 13, 2025

Deploy Preview for meltano-hub ready!

Name Link
🔨 Latest commit c837fbf
🔍 Latest deploy log https://app.netlify.com/sites/meltano-hub/deploys/67ae30a4df196f0008ce5c6b
😎 Deploy Preview https://deploy-preview-1948--meltano-hub.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

github-actions bot commented Feb 13, 2025

Testing plugin tap-fixerio (singer-io variant):

Starting test job...
...
Job completed.

Usage info
melty-bot % tap-fixerio --help
usage: tap-fixerio [-h] -c CONFIG [-s STATE] [-p PROPERTIES]
                   [--catalog CATALOG] [-d]

options:
  -h, --help            show this help message and exit
  -c CONFIG, --config CONFIG
                        Config file
  -s STATE, --state STATE
                        State file
  -p PROPERTIES, --properties PROPERTIES
                        Property selections: DEPRECATED, Please use --catalog
                        instead
  --catalog CATALOG     Catalog file
  -d, --discover        Do schema discovery
Detected capabilities
  • ✅ 'discover'
  • ✅ 'catalog'
  • ✅ 'state'
  • ❌ 'about'

@edgarrmondragon
Copy link
Collaborator Author

edgarrmondragon commented Feb 13, 2025

Ok, this is weird. The tap's source doesn't have --discover and --properties, but pulling it from PyPI does:

$ uvx -p 3.9 tap-fixerio --help
Installed 13 packages in 10ms
usage: tap-fixerio [-h] -c CONFIG [-s STATE] [-p PROPERTIES] [--catalog CATALOG] [-d]

optional arguments:
  -h, --help            show this help message and exit
  -c CONFIG, --config CONFIG
                        Config file
  -s STATE, --state STATE
                        State file
  -p PROPERTIES, --properties PROPERTIES
                        Property selections: DEPRECATED, Please use --catalog instead
  --catalog CATALOG     Catalog file
  -d, --discover        Do schema discovery

Source:

https://github.com/singer-io/tap-fixerio/blob/7f26965b36fa74ad0b31420fe49439f9dec47813/tap_fixerio.py#L88-L96

@edgarrmondragon
Copy link
Collaborator Author

edgarrmondragon commented Feb 13, 2025

Oh, it's because the tap is using singer.utils.parse_args, which adds all the Singer standard CLI options: https://github.com/singer-io/singer-python/blob/ae50276b7055248273d0458b85d891773f7d4597/singer/utils.py#L126.

The tap doesn't use anything other than --config/-c and --state/-s though, so this PR is correct.

@edgarrmondragon
Copy link
Collaborator Author

singer-io/tap-fixerio#15.

@edgarrmondragon edgarrmondragon merged commit 322a439 into main Feb 13, 2025
14 checks passed
@edgarrmondragon edgarrmondragon deleted the edgarrmondragon-patch-1 branch February 13, 2025 18:12
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.

1 participant