Open dluftspring opened 1 year ago
@edgarrmondragon is this related / similar to the Decimal bugs we were seeing?
@edgarrmondragon is this related / similar to the Decimal bugs we were seeing?
@tayloramurphy nope, this is a decision made very early in the sdk history to add T00:00:00+00:00
to all date values: https://github.com/meltano/sdk/blob/b02d698c15c1fff8667e18e5301c65a0fd0247b5/singer_sdk/helpers/_typing.py#L478
I don't recall why that's the case, and I don't know if removing it would result in a breaking change for any taps/targets.
@edgarrmondragon this is a real Chesterton's fence situation... it doesn't really make sense to me why it would coerce a date to a datetime, particularly when those are distinct types. Thoughts on what a possible mitigation would be? Should this be configurable by the end user?
https://github.com/MeltanoLabs/tap-postgres/pull/172/files One possible implementation to fix it :D
@tayloramurphy @edgarrmondragon I also just ran into this when using tap-snowflake and created https://github.com/MeltanoLabs/tap-snowflake/issues/26 before I knew this issue existed. It looks like this is the old MR that added it originally https://gitlab.com/meltano/sdk/-/merge_requests/42 but its a giant release MR so its hard to grok.
I think a reasonable solution inspired by Derek's implementation would be to encapsulate all the conforming methods into a dedicated class, add it to the base stream class, and allow tap developers to override the primitive-level conforming method.
I think I have a solution for this that
1) Maintains backwards compatibility with whatever relied on this originally (Chesterton's fence) 2) Makes type coercion easily extensible so extractors can manage their own
The implementation consists on refactoring _conform_primitive_property
using functools.singledispatch
.
I ran into this issue as we are trying to migrate our integrations at Akkio to Meltano. I am trying to migrate snowflake and got an error when uploading to target-s3 from tap-snowflake since date type is expected. I am considering a patch similar to https://github.com/MeltanoLabs/tap-postgres/pull/172/files where we override _conform_primitive_property for now.
Singer SDK Version
0.30.0
Meltano version 2.19.1
Is this a regression?
Python Version
3.10
Bug scope
Targets (data type handling, batching, SQL object generation, etc.)
Operating System
Linux, Mac OS
Description
When syncing data between
tap-snowflake
andtarget-postgres
this line causes JSON schema validation to fail because the inferred json schema from Snowflake is a date but this coerces the type into a datetime.To summarize
date
in Snowflakedate
in Postgresformat: string type: date
and instead receivesformat: string type: date-time
.Why is that extra timestamp string being added on?
Additionally you can verify that this is a problem by syncing any snowflake date colum into target-jsonl and see that it is being serialized as a timestamp instead of a regular date even though
produces perfectly valid JSON. There's probably a reason that little string is added where it is but it does cause some problems for Meltano.
Code