swagger-api / swagger-api/swagger-codegen

[PYTHON] Python codegen should use isoparse instead of parse for datetime

Open
#11,278 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Mustache
Stars
17.8k
Forks
6k
PR merge metrics
No merged PRs in 30d

Description

Description

Right now, when parsing a datetime (date-time in swagger), Python uses dateutil.parser.parse. This has led to some weirdness in a subset of our environments where the timezone will be set as "tzlocal" instead of "tzutc", as those are analogous in our environments that are set to UTC. Since our environments run remotely, it can be confusing as to what "tzlocal" is.

Reference: https://github.com/swagger-api/swagger-codegen/blob/751e59df060b1c3ecf54921e104f2086dfa9f820/modules/swagger-codegen/src/main/resources/python/api_client.mustache#L596

Swagger's date-time field is documented here https://swagger.io/docs/specification/data-models/data-types/#format to follow the specification of "RFC 3339, section 5.6" (https://tools.ietf.org/html/rfc3339#section-5.6), which follows the ISO-8601 format. Therefore I see no official issue with substituting isoparse in-place of parse. Further, the docstring in the mustache template specifies that the input should be ISO-8601 formatted.

However, since dateutil.parser.parse is a less-strict parser, it may accept non-ISO-8601 strings without additional configuration, for example "20-JUN-1990 08:03:00". Using such a string for the date-time format is against the official swagger spec, but it may be in use, and this change would then be considered a backwards-incompatible change as the generated client will no longer accept non-ISO-8601 strings. Would love to hear the opinions of the community on this one.

Swagger-codegen version

We see this in 3.0.22, though the code is still using datetime.parser.parse in master

Swagger declaration file content or url

Occurs when using a date-time field, e.g.

"created_at": {
"type": "string",
"format": "date-time",
"description": "When the application was created."
},

Command line used for generation

API_VERSION="$(date +"%y.%m.%d")"
PYTHON_SWAGGER="/path/to/python.swagger.json"
java -jar swagger-codegen-cli-3.0.22 generate -i ${PYTHON_SWAGGER} -l python -c config.json -o python_api/ --additional-properties=packageVersion=${API_VERSION}

(config.json just sets the packageName, projectName, and default packageVersion which is overridden)

Steps to reproduce

Create a swagger file containing a date-time field
Run the codegenerator for python to create the python fixtures
Check api_client.py and look for __deserialize_datatime, and see that it uses datetime.parser.parse

(apologies for this repro being on-the-nose, I don't have a good RCA for why datetime.parser.parse handles the timezone weirdly while isoparse does not, but even outside the timezone, isoparse is faster!).

Related issues/PRs

Could not find any

Suggest a fix/enhancement

Change this line:

https://github.com/swagger-api/swagger-codegen/blob/751e59df060b1c3ecf54921e104f2086dfa9f820/modules/swagger-codegen/src/main/resources/python/api_client.mustache#L596

to use isoparse instead of parse

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with modules/swagger-codegen/src/main/resources/python/api_client.mustache at the referenced datetime deserialization line, then inspect the generated api_client.py from a date-time fixture. Check the RFC 3339/ISO-8601 behavior and the compatibility concern around non-ISO inputs. Done means the template produces the requested parser behavior without leaving the generated Python client inconsistent.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.