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

Closed
thosil opened this issue Sep 28, 2023 · 3 comments
Assignees
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for 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
Contributor

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
@jeremystretch jeremystretch self-assigned this Jun 14, 2024
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jun 14, 2024
@jeremystretch
Copy link
Member

jeremystretch commented Jun 14, 2024

It seems the root issue is that Django REST Framework has taken it upon itself, for reasons unknown to me, to arbitrarily replace the +00:00 suffix of a timestamp with Z (for Zulu).

We can work around this for custom fields for now by replacing the Z suffix with the zero offset on incoming data.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for 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