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

Represent SafeString as plain string on schema rendering. #8429

Merged
merged 3 commits into from
Nov 23, 2022

Conversation

hashlash
Copy link
Contributor

Note: Before submitting this pull request, please review our contributing guidelines.

Description

Fix #8428.

@hashlash hashlash force-pushed the yaml-representer-fix branch from 18581ae to ac6f84e Compare March 25, 2022 02:15
@hashlash hashlash marked this pull request as ready for review March 25, 2022 02:15
@hashlash
Copy link
Contributor Author

I tried running tox -e docs on branch master, and it also failed (with the same error). So the problem shouldn't be from the changes in this PR.

Should we wait for the fix, or can we merge it first?

@hashlash
Copy link
Contributor Author

The source of error: mkdocs/mkdocs#2799

@hashlash hashlash force-pushed the yaml-representer-fix branch from ac6f84e to ed3baea Compare March 26, 2022 23:24
@hashlash
Copy link
Contributor Author

The error should be fixed in #8433.

@stale
Copy link

stale bot commented Jun 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 13, 2022
@hashlash hashlash force-pushed the yaml-representer-fix branch from d05e111 to 20f7671 Compare June 13, 2022 14:21
@stale stale bot removed the stale label Jun 13, 2022
@stale
Copy link

stale bot commented Aug 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 13, 2022
@auvipy auvipy requested a review from adamchainz November 22, 2022 07:42
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have much depth regarding yaml. will be waiting for more eyes into this. on a side note, can you add more relevant tests for the changes?

@hashlash
Copy link
Contributor Author

hashlash commented Nov 23, 2022

can you add more relevant tests for the changes?

@auvipy I think the test is sufficient. As long as it can render SafeString, I think it's good enough.

Or could you elaborate on what kind of test I should add?

@auvipy auvipy requested a review from tfranzel November 23, 2022 13:54
@auvipy
Copy link
Member

auvipy commented Nov 23, 2022

I have requested review from another OpenAPI specialist

@auvipy auvipy removed the request for review from adamchainz November 23, 2022 13:55
@hashlash
Copy link
Contributor Author

hashlash commented Nov 23, 2022

Btw, here's the section in PyYAML that discuss about representing custom object: https://pyyaml.org/wiki/PyYAMLDocumentation#constructors-representers-resolvers

@stale stale bot removed the stale label Nov 23, 2022
@auvipy auvipy added the Bug label Nov 23, 2022
@auvipy auvipy merged commit ebde56b into encode:master Nov 23, 2022
Copy link
Member

@tfranzel tfranzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that works. It is key that the yaml does not contain !! type injections, which are valid in newer YAML versions, but upstream tools mostly choke on them.

But just as a sidenote: there are a bunch more objects (e.g uuid, datetime, etc) that solicit this kind of behavior: https://github.com/tfranzel/drf-spectacular/blob/03677b6bb86b2076a97b70b918ed3b3b528edaa4/drf_spectacular/renderers.py#L52

@hashlash
Copy link
Contributor Author

Oops, you're right. I just read the discussion #8453, but wasn't aware with the drf-spectacular renderer implementation. Based on the discussion, going forward, the renderer part will also be deprecated from rest_framework?

@hashlash hashlash deleted the yaml-representer-fix branch November 23, 2022 15:38
@tfranzel
Copy link
Member

@hashlash, yes. everything that does not belong to the schema interface itself will be retired and removed eventually.

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

Successfully merging this pull request may close these issues.

Unknown tag on redered schema when using Django's SafeString as help_text
3 participants