search

microsoft / kiota-python

Abstractions library for Kiota generated Python clients
https://aka.ms/kiota/docs
MIT License
14 stars 10 forks source link

RFC 3339 + Python #352

Open pjmagee opened 1 month ago

pjmagee commented 1 month ago

What are you generating using Kiota, clients or plugins?

API Client/SDK

In what context or format are you using Kiota?

Nuget tool

Client library/SDK language

Python

Describe the bug

Python Serialized date: 2024-09-11T03:33:16+00:00Z

Link to Microsoft ISO/RFC 3339 for .NET:

https://learn.microsoft.com/en-us/dotnet/standard/datetime/system-text-json-support#the-extended-iso-8601-12019-profile-in-systemtextjson

When the Python SDK sends a request to an API using .NET System.Text.Json to Parse and validate RFC 3339 from a Kiota Python SDK datetime, it cannot deserialize the datetime.

If the Python code omitted the Z at the end, it would be valid because including the +00:00 timezone already means its UTC.

From the Microsoft page on RFC Parsing (for .NET at least)

4. "'Full date''T''Time hour'':''Minute''Time offset'"
"yyyy'-'MM'-'dd'T'HH':'mmZ"
"yyyy'-'MM'-'dd'T'HH':'mm('+'/'-')HH':'mm"

Check the link on RFC 3339 section 5.6:

https://datatracker.ietf.org/doc/html/rfc3339#section-5.6

It states Z OR time-numoffset

Expected behavior

2024-09-11T03:33:16+00:00Z should be serialised as 2024-09-11T03:33:16+00:00

How to reproduce

Any openapi spec with a datetime sent to a server to parse RFC 3339 datetime.

Using .NET System.Text.Json RFC 3339 Support

Open API description file

No response

Kiota Version

1.18.0

Latest Kiota version known to work for scenario above?(Not required)

No response

Known Workarounds

No response

Configuration

No response

Debug output

Click to expand log ``` ```

Other information

No response

pjmagee commented 1 month ago 
>>> datetime.datetime.now().astimezone().isoformat()
'2024-09-11T12:29:05.080734+01:00'

This looks to be a valid RFC outcome, but it needs astimezone(). A Z at the end of this would invalidate RFC Specification

pjmagee commented 1 month ago

I'm going to debug shortly and see if i can find the line of the python lib which may need reviewing. According to the spec timezone+Z is invalid.

baywet commented 1 month ago

Hi @pjmagee Thank you for using kiota and for reaching out.

Let's use RFC3339 moving forward in this discussion since it's the standard that OAS uses

If I understand your issue properly:

Does that reflect the situation properly?

If so, I've transferred this issue to the Python serialization repository, this is most likely caused by this block

Unit tests are implemented here (but only for the string type).

Is this something you'd like to submit a pull request for provided some guidance?

pjmagee commented 1 month ago

Hi @pjmagee Thank you for using kiota and for reaching out.

Let's use RFC3339 moving forward in this discussion since it's the standard that OAS uses

If I understand your issue properly:

  • When serializing a DateTimeOffset in dotnet which is on the UTC standard you get +00:00 or Z at the end, but not both
  • When serializing a datetime in pythonwhich is on the UTC standard you get +00:00Z at the end, which is incorrect according to RFC3339

Does that reflect the situation properly?

If so, I've transferred this issue to the Python serialization repository, this is most likely caused by this block

Unit tests are implemented here (but only for the string type).

Is this something you'd like to submit a pull request for provided some guidance?

Yes, this seems to be what i find occurring. I also am using System.Text.Json to 'deserialise' RFC 3339 Dates as our server is .NET, but I am using Kiota 'examples' in various languages (go, python, c#) etc.

I am happy to try and raise a PR, similar to the other date issue i fixed a while back.

baywet commented 1 month ago

Let us know if you have any additional comments or questions.

pjmagee commented 1 month ago

I think i found the bug.... It's from stnduritemplate in the __init__.py of the package.

    @staticmethod
    def __convert_native_types(value: Any) -> str:
        if isinstance(value, (str)):
            return value
        if isinstance(value, (bool)):
            return str(value).lower()
        elif isinstance(value, (int, float)):
            return str(value)
        elif isinstance(value, datetime):
            return value.isoformat("T") + "Z"

This Z is forced, this is not RFC 3339 Compliant.

I think the fix is:

if value.tzinfo is None:
    return value.isoformat("T") + "Z"
else:
   return value.isoformat("T")
baywet commented 1 month ago

Thank you for the additional information. I've seen multiple notifications come in, this is the first one I'm looking at, this answer might be a bit behind.

There are effectively multiple places where the format of a date time matters:

As a general rule of thumb: kiota client should try to normalize URI parameters as much as it can before sending it down to std uri template. This is because we have more context and dependencies than this lib has and can handle more data types it can.

We should also strive to have the same implementation for date time across abstractions and serialization implementations to achieve maximal consistency.

microsoft-github-policy-service[bot] commented 1 month ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.