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

Add a timezone-aware datetime GraphQL scalar type: DateTimeTz #829

Open
obi1kenobi opened this issue May 19, 2020 · 2 comments
Open

Add a timezone-aware datetime GraphQL scalar type: DateTimeTz #829

obi1kenobi opened this issue May 19, 2020 · 2 comments

Comments

@obi1kenobi
Copy link
Contributor

Since #827 merged, we don't have a way to represent timezone-aware datetimes. For this, I propose adding a new scalar type: scalar DateTimeTz, Python name GraphQLDateTimeTz.

A few guidelines I'd like to propose:

  • Outputting a field of type DateTimeTz is guaranteed to produce a timezone-aware result.
  • Runtime arguments of type DateTimeTz are required to contain timezone information.
  • Its serialization as a string must always contain either an explicit +HH:mm suffix or the simple suffix Z which is equivalent to +00:00. Other suffixes, such as +HH, +H, America/New_York etc. are explicitly not permitted and unsupported.
  • When auto-generating schemas, if a datetime type is explicitly known to carry timezone information (for example, TIMESTAMPTZ in many SQL flavors), it must be represented as DateTimeTz in the auto-generated schema.
  • When auto-generating schemas, if it is uncertain whether a datetime type carries timezone information or not, it must be represented as DateTime (i.e. timezone-naive) and a warning must be emitted about the uncertainty. This warning must provide sufficient information to the user so that they are able to manually resolve the issue by explicitly configuring the field in question to appear as timezone-aware or naive.
@pmantica1
Copy link
Contributor

pmantica1 commented May 19, 2020

This all sounds good to me. I'd like to mention a couple of points in support of this:
3. I think that the guideline you described here sticks completely with ISO standard and that makes sense to me. Sticking to the ISO standard makes the documentation and code simpler.
5. We already have some functionality to support this. We could tell the user to use class_to_field_type_overrides in get_sqlalchemy_schema_info and get_graphql_schema_from_orientdb_schema_data to override the type in the GraphQL schema.

@obi1kenobi
Copy link
Contributor Author

  1. I think that the guideline you described here sticks completely with ISO standard and that makes sense to me. Sticking to the ISO standard makes the documentation and code simpler.

Yes, exactly. And specifically, this is narrowing the ISO 8601 standard, which is huge — we don't have the resources to implement or support the whole thing.

  1. We already have some functionality to support this. We could tell the user to use class_to_field_type_overrides in get_sqlalchemy_schema_info and get_graphql_schema_from_orientdb_schema_data to override the type in the GraphQL schema.

Yes — I just wanted to make sure we don't frame the guidelines I proposed in terms of existing implementations or existing backend implementations. I'm fine with using existing code to achieve these goals.

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

No branches or pull requests

2 participants