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

[Bug]: TypeError thrown by SQLConnector child class generated by cookie cutter #967

Closed
kgpayne opened this issue Sep 14, 2022 · 5 comments · Fixed by #972
Closed

[Bug]: TypeError thrown by SQLConnector child class generated by cookie cutter #967

kgpayne opened this issue Sep 14, 2022 · 5 comments · Fixed by #972
Assignees
Labels
kind/Bug Something isn't working valuestream/SDK

Comments

@kgpayne
Copy link
Contributor

kgpayne commented Sep 14, 2022

Singer SDK Version

0.10.0

Python Version

3.8

Bug scope

Taps (catalog, state, stream maps, etc.)

Operating System

macOS

Description

Working on tap-snowflake I have run into an issue whereby the default to_jsonschema_type static method on the generated SnowflakeConnector class, which simply calls super().to_jsonschema_type(...) raises a TypeError on super(). Removing the method on the child resolves the issue, however the child method is only safe to remove if no custom mappings are required.

Taking a look online, I found this SO answer that seems to match this case.

Code

Traceback (most recent call last):
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/bin/tap-snowflake", line 8, in <module>
    sys.exit(TapSnowflake.cli())
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/tap_base.py", line 489, in cli
    tap = cls(  # type: ignore  # Ignore 'type not callable'
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/tap_base.py", line 540, in __init__
    super().__init__(
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/tap_base.py", line 95, in __init__
    self.mapper.register_raw_streams_from_catalog(self.catalog)
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/tap_base.py", line 157, in catalog
    self._catalog = self.input_catalog or self._singer_catalog
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/tap_base.py", line 249, in _singer_catalog
    for stream in self.streams.values()
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/tap_base.py", line 120, in streams
    for stream in self.load_streams():
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/tap_base.py", line 281, in load_streams
    for stream in self.discover_streams():
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/tap_base.py", line 576, in discover_streams
    for catalog_entry in self.catalog_dict["streams"]:
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/tap_base.py", line 564, in catalog_dict
    result["streams"].extend(connector.discover_catalog_entries())
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/streams/sql.py", line 438, in discover_catalog_entries
    catalog_entry = self.discover_catalog_entry(
  File "/Users/kp/Projects/singer/tap-snowflake/.meltano/extractors/tap-snowflake/venv/lib/python3.8/site-packages/singer_sdk/streams/sql.py", line 379, in discover_catalog_entry
    jsonschema_type: dict = self.to_jsonschema_type(
  File "/Users/kp/Projects/singer/tap-snowflake/tap_snowflake/client.py", line 39, in to_jsonschema_type
    return super().to_jsonschema_type(sql_type)
TypeError: super(type, obj): obj must be an instance or subtype of type
@kgpayne kgpayne added kind/Bug Something isn't working valuestream/SDK labels Sep 14, 2022
@BuzzCutNorman
Copy link
Contributor

BuzzCutNorman commented Sep 14, 2022

I don't know if this is a proper python way of dealing with the issue but I have been changing the call to return SQLConnector.to_jsonschema_type(sql_type) and it seem to work well. I found the same issue with to_sql_type and have changed it to return SQLConnector.to_sql_type(jsonschema_type).

I have not played with this thought but I have always wonder if it might be this. The method in the client.py returns a type of sqlalchemy.types.TypeEngine: While in the stream sql.py version it says it returns a type of dict.

client.py

@staticmethod
    def to_jsonschema_type(sql_type: dict) -> sqlalchemy.types.TypeEngine:

stream sql.py

    @staticmethod
    def to_jsonschema_type(
        sql_type: (
            str | sqlalchemy.types.TypeEngine | type[sqlalchemy.types.TypeEngine] | Any
        ),
    ) -> dict:

Nope that wasn't it I changed the client.py version to def to_jsonschema_type(sql_type: dict) -> dict: and got the same typing error.

@kgpayne
Copy link
Contributor Author

kgpayne commented Sep 14, 2022

Thanks @BuzzCutNorman that is super helpful! Given that the method is static, I don't see why using the plain class directly, in stead of via the super() factory, would be in any way improper. If @edgarrmondragon or anyone else from @meltano/engineering concur I will raise a PR to change these references in the cookiecutters 🙂

@edgarrmondragon
Copy link
Collaborator

@kgpayne yeah, calling the static on the class itself sounds good. Go for it 😄

@BuzzCutNorman
Copy link
Contributor

Sweet glad to hear that is a possible solution. Following the links in the stackoverflow issue given earlier I was trying the __init__ solution mentioned in this blog post Another super() wrinkle – raising TypeError and was not able to quickly find a combination that resolved the issue like it did for the author.

@aaronsteers
Copy link
Contributor

@kgpayne yeah, calling the static on the class itself sounds good. Go for it 😄

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/Bug Something isn't working valuestream/SDK
Projects
None yet
4 participants