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

Quick fix for AnyType ruby bindings #3679

Merged
merged 1 commit into from
Jul 10, 2024
Merged

Quick fix for AnyType ruby bindings #3679

merged 1 commit into from
Jul 10, 2024

Conversation

pedro-psb
Copy link
Member

@pedro-psb pedro-psb commented Jul 9, 2024

Quicker alternative to #3653 for fixing #3639

I must add that this is not the "correct way" of fixing it, but its quick and safe (in the sense that it leaves us in a similar state we had before, where all jsonfields were recognized as Objects). Its not the correct way because:

  • the schema type is misleading, which is not helpful for users. E.g, its says its an Object (openapi type) when its an Array example
  • its not doing basic structural validation (often we know exactly what the json structure is)

Reference

@github-actions github-actions bot added the multi-commit Added when a PR consists of more than one commit label Jul 9, 2024
The JSONField was yielding an empty type on the schema, which
broke bindings generation.

This replaces drf serializers.JSONField with a custom one that is an
OpenApi 'object' type.

Closes: pulp#3639
@github-actions github-actions bot removed the multi-commit Added when a PR consists of more than one commit label Jul 9, 2024
@pedro-psb
Copy link
Member Author

Given the diff in the clients specs, the unexpected presence of the AnyType in the ruby client is the only cause of the problem.

We don't test the bindings on PR, so to assert that manually you can:

  • install the local PR checkout in a venv
  • run pulpcore-manager openapi --bindings --component rpm --file rpm-api.json
  • move it to a recent pulp-openapi-generator repo local checkout
  • run ./gen-client.sh rpm-api.json rpm ruby pulp_rpm
  • grep inside pulp_rpm-ruby-client to assert there is no AnyType anywhere

@dralley
Copy link
Contributor

dralley commented Jul 10, 2024

So this seems reasonable to me, but @mdellweg is probably a better judge of correctness. I'd like us to file an issue to revert this change once a "correct" fix is ready though

Copy link
Member

@mdellweg mdellweg left a comment

Choose a reason for hiding this comment

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

Yes, the would work in isolation for these pulp_rpm fields.

Maybe you want to call it JSONObjectField.

@ggainey ggainey merged commit 2e0c293 into pulp:main Jul 10, 2024
16 checks passed
Copy link

patchback bot commented Jul 10, 2024

Backport to 3.27: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.27/2e0c2933675c0ce41d8ac5c801f8d0dbc54349d8/pr-3679

Backported as #3680

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

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

Successfully merging this pull request may close these issues.

4 participants