Add typing necessary to satisfy eda-server OpenAPI issues #276
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
relrod
reviewed
Apr 6, 2024
AlanCoding
added
the
Ready for review
Dostonbek1
approved these changes
Apr 8, 2024
There was a problem hiding this comment.
I tested the changes in this PR in combination with the PR on eda-server and confirmed that it resolves the outstanding warnings related to dab_rbac library. The only remaining warnings are related to resource_registry (and one native to eda-server), which I'm not sure if it falls in the scope of this PR. Remaining warnings for ref:
eda-api-1 | /app/src/src/aap_eda/api/serializers/rulebook.py: Warning [AuditRuleViewSet > AuditEventSerializer]: could not resolve serializer field "YAMLSerializerField(allow_null=True, help_text='The payload in the event', required=False)". Defaulting to "string"
eda-api-1 | /app/venv/lib64/python3.11/site-packages/ansible_base/lib/routers/association_resource_router.py: Warning [RelatedRoleTeamAssignmentViewSet]: could not derive type of path parameter "team_assignments" because it is untyped and obtaining queryset from the viewset failed. Consider adding a type to the path (e.g. <int:team_assignments>) or annotating the parameter type with @extend_schema. Defaulting to "string".
eda-api-1 | /app/venv/lib64/python3.11/site-packages/ansible_base/lib/routers/association_resource_router.py: Warning [RelatedRoleUserAssignmentViewSet]: could not derive type of path parameter "user_assignments" because it is untyped and obtaining queryset from the viewset failed. Consider adding a type to the path (e.g. <int:user_assignments>) or annotating the parameter type with @extend_schema. Defaulting to "string".
eda-api-1 | /app/venv/lib64/python3.11/site-packages/ansible_base/resource_registry/views.py: Error [ServiceIndexRootView]: exception raised while getting serializer. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: 'super' object has no attribute 'get_serializer_class')
eda-api-1 | /app/venv/lib64/python3.11/site-packages/ansible_base/resource_registry/views.py: Error [ServiceMetadataView]: exception raised while getting serializer. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: 'super' object has no attribute 'get_serializer_class')
eda-api-1 | /app/venv/lib64/python3.11/site-packages/ansible_base/resource_registry/serializers.py: Warning [ResourceTypeViewSet > ResourceTypeSerializer]: unable to resolve type hint for function "get_url". Consider using a type hint or @extend_schema_field. Defaulting to string.
eda-api-1 | /app/venv/lib64/python3.11/site-packages/ansible_base/resource_registry/serializers.py: Warning [ResourceViewSet > ResourceListSerializer]: unable to resolve type hint for function "get_url". Consider using a type hint or @extend_schema_field. Defaulting to string.
eda-api-1 | /app/venv/lib64/python3.11/site-packages/ansible_base/resource_registry/serializers.py: Warning [ResourceViewSet > ResourceSerializer]: unable to resolve type hint for function "get_url". Consider using a type hint or @extend_schema_field. Defaulting to string.
eda-api-1 | /app/venv/lib64/python3.11/site-packages/ansible_base/resource_registry/views.py: Error [ValidateLocalUserView]: exception raised while getting serializer. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: 'super' object has no attribute 'get_serializer_class')
|
Oh this isn't good:
I'll push a fix for that (in the eda-server PR). I thought some others would be resolved like:
I'll have one more quick look, because I thought those were fixed. |
It seems this is because of the 2 potential return types. I'm not sure if we can convert the return value to string? Would that be okay? |
|
I filed #283 for the resource registry endpoints. |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes problems that I encountered on the eda-server RBAC branch using the OpenAPI generator, when rendering the
openapi.jsonendpoint.For these things, it complains that it doesn't know what type to use. It hasn't stopped complaining. Here are some that still remain, limited to DAB content:
But the much larger list of issues is knocked down by these changes.
The generated client also somewhat needs this to be right, and this matters most for the related endpoint
pkparameter which it could not identify as an integer before. So that in specific is likely to fix a bug their QE hasn't gotten far enough to hit yet.Type hints are quite nice for this, in that it avoids us having to add the dependency and decorate all our views, and I really don't want to do that.