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

format of date_time custom fields through the REST API on GET are not compatible with POST #13925

Open
thosil opened this issue Sep 28, 2023 · 2 comments
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@thosil
Copy link

thosil commented Sep 28, 2023

NetBox version

v3.5.4

Python version

3.10

Steps to Reproduce

  1. make sure timezone is set to UTC
  2. create a custom field of type date_time (let's say "my_dt")
  3. set a value using the nebtox UI
  4. get the value with the REST API (will be in format "YYYY-mm-ddTHH:MM:SSZ")
  5. POST a new value using the same format (with the trailing Z) or modify any other custom_field of the object → netbox does not validate the format of my_dt

Expected Behavior

The input format of datetime should be compatible with the output format.

Observed Behavior

We receive this error:

Invalid value for custom field 'start_prep': Date and time values must be in ISO 8601 format (YYYY-MM-DD HH:MM:SS).

Correct me if I'm wrong, but the values observed through the API are serialized with django rest_framework, which could explain why there's a different format between the rest api and inside the database. But when we do a post, the validation go through extras/models/customfields.py and use datetime.fromisoformat() (also for deserialisation).

Starting python 3.11 fromisoformat() is compatible with the YYYY-mm-ddTHH:MM:SSZ format, but not former version (see changed in version 3.11 in the doc). But it's not always easy to upgrade python.

I patched my version with dateutil.parser.parse() instead of datetime.fromisoformat() and it's working as expected. Should I make a PR?

Thanks

@thosil thosil added the type: bug A confirmed report of unexpected behavior in the application label Sep 28, 2023
@arthanson arthanson added the severity: low Does not significantly disrupt application functionality, or a workaround is available label Sep 28, 2023
@jeremystretch
Copy link
Member

Unfortunately this is a limitation of the datetime.fromisoformat() in Python < 3.11. This StackOverflow question has a lot of discussion around it. (However, I'm not a fan of introducing a new dependency just to support this.)

We'll need to come up with a way to support multiple conforming date/time formats.

@jeremystretch jeremystretch added the status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation label Dec 26, 2023
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Mar 26, 2024
@arthanson arthanson self-assigned this Apr 19, 2024
@arthanson arthanson removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation pending closure Requires immediate attention to avoid being closed for inactivity labels Apr 19, 2024
@jeremystretch jeremystretch added the status: accepted This issue has been accepted for implementation label May 13, 2024
@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: accepted This issue has been accepted for implementation labels May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants