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(templates): Tap template: fix style and docstrings, and add test cases for SQL and "Other" sources #1434

Merged

Conversation

flexponsive
Copy link
Contributor

@flexponsive flexponsive commented Feb 20, 2023

Follow-up to #1410: Now all tap templates are covered by e2e tests

  • Add replay file for tap from SQL source and Other source
  • Fix mypy + docstring errors

📚 Documentation preview 📚: https://meltano-sdk--1434.org.readthedocs.build/en/1434/

tap_sql_custom/tap.py:8:1: F401 'tap_sql_custom.client.AutomaticTestTapStream' imported but unused
tap_sql_custom/client.py:16:1: DAR101 Missing parameter(s) in Docstring: - config
tap_sql_custom/client.py:16:1: DAR201 Missing "Returns" in Docstring: - return
tap_sql_custom/client.py:29:1: DAR201 Missing "Returns" in Docstring: - return
tap_sql_custom/client.py:29:1: DAR101 Missing parameter(s) in Docstring: - sql_type
tap_sql_custom/client.py:40:1: DAR101 Missing parameter(s) in Docstring: - jsonschema_type
tap_sql_custom/client.py:40:1: DAR201 Missing "Returns" in Docstring: - return
tap_sql_custom/client.py:54:38: F821 undefined name 'Optional'
tap_sql_custom/client.py:54:57: F821 undefined name 'Iterable'
tap_sql_custom/client.py:54:66: F821 undefined name 'Dict'
tap_sql_custom/client.py:54:76: F821 undefined name 'Any'
@codecov
Copy link

codecov bot commented Feb 20, 2023

Codecov Report

Merging #1434 (f2a937d) into main (6700e9e) will not change coverage.
The diff coverage is n/a.

❗ Current head f2a937d differs from pull request most recent head 924785c. Consider uploading reports for the commit 924785c to get more accurate results

@@           Coverage Diff           @@
##             main    #1434   +/-   ##
=======================================
  Coverage   85.47%   85.47%           
=======================================
  Files          57       57           
  Lines        4763     4763           
  Branches      808      808           
=======================================
  Hits         4071     4071           
  Misses        501      501           
  Partials      191      191           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@flexponsive flexponsive changed the title fix(templates): e2e testing - tap SQL fix(templates): e2e testing - tap SQL & Other Feb 20, 2023
@flexponsive flexponsive marked this pull request as ready for review February 20, 2023 19:31
@tayloramurphy
Copy link
Collaborator

Thanks for the PR @flexponsive - looks like we've got some engineers on it as reviewers 👍

Copy link
Collaborator

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

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

@flexponsive should we add these new cases to the GitHub workflow file?

@flexponsive
Copy link
Contributor Author

@edgarrmondragon good catch! thanks for pointing it out. added the missing cases

@edgarrmondragon edgarrmondragon changed the title fix(templates): e2e testing - tap SQL & Other fix(templates): Tap template: fix style and docstrings, and add test cases for SQL and "Other" sources Feb 21, 2023
@edgarrmondragon
Copy link
Collaborator

thanks @flexponsive!

@edgarrmondragon edgarrmondragon merged commit 53c3fcd into meltano:main Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants