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]: Improve error message when replication key is not present in schema #1332

Closed
edgarrmondragon opened this issue Jan 18, 2023 · 4 comments · Fixed by #1927
Closed

[Bug]: Improve error message when replication key is not present in schema #1332

edgarrmondragon opened this issue Jan 18, 2023 · 4 comments · Fixed by #1927
Labels

Comments

@edgarrmondragon
Copy link
Collaborator

Singer SDK Version

0.17.0

Python Version

NA

Bug scope

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

Operating System

NA

Description

Right now, users only get the following rather unhelpful message:

ValueError: Could not detect type from empty type_dict. Did you forget to define a property in the stream schema?

Maybe the stream could catch ValueError and resurface it with a better message that contains the name of the field.

Code

No response

@stale
Copy link

stale bot commented Jul 18, 2023

This has been marked as stale because it is unassigned, and has not had recent activity. It will be closed after 21 days if no further activity occurs. If this should never go stale, please add the evergreen label, or request that it be added.

@stale stale bot added the stale label Jul 18, 2023
@edgarrmondragon
Copy link
Collaborator Author

Still relevant

@stale stale bot removed the stale label Jul 20, 2023
@mjsqu
Copy link
Contributor

mjsqu commented Aug 27, 2023

Interested in working on this one, just going to jot some notes:

  • The message comes from singer_sdk.helpers._typing.EmptySchemaTypeError
  • This error is raised in is_datetime_type, called fromsinger_sdk.streams.core.is_timestamp_replication_key:
    @property
    def is_timestamp_replication_key(self) -> bool:
        """Check is replication key is a timestamp.

        Developers can override to `True` in order to force this value, although this
        should not be required in most use cases since the type can generally be
        accurately detected from the JSON Schema.

        Returns:
            True if the stream uses a timestamp-based replication key.
        """
        if not self.replication_key:
            return False
        type_dict = self.schema.get("properties", {}).get(self.replication_key)
        return is_datetime_type(type_dict)

@mjsqu
Copy link
Contributor

mjsqu commented Aug 27, 2023

Suggested steps to fix:

  • add a new Exception to singer_sdk.exceptions, or leverage an existing one if it exists
  • check the value of type_dict in the function is_timestamp_replication_key and raise the new Exception if None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants