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

Json2code: full $ref support #2452

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bashtanov
Copy link

Swagger only allows to refer to custom composite types via "$ref", not via "type", whether it is about array elements or individual items.

Disallowing "type" would break existing use, so allow "$ref" to be used interchangeably.

Swagger only allows to refer to custom composite types via "$ref",
not via "type", whether it is about array elements or individual items.

Disallowing "type" would break existing use, so allow "$ref" to
be used interchangeably.

Signed-off-by: Alexey Bashtanov <[email protected]>
bashtanov added a commit to bashtanov/redpanda that referenced this pull request Sep 24, 2024
Swagger needs "$ref", while seastar only allows "type" (at least until
scylladb/seastar#2452 is merged), so let's use both
for now
@tchaikov tchaikov self-requested a review September 24, 2024 13:42
@tchaikov
Copy link
Contributor

@amnonh hi Amnon, could you take a look at this change as well? i plan to review it in this week.

Comment on lines +373 to +379
try:
type = getitem(properties[var], "type", name + ":" + var)
except Exception as e:
try:
type = getitem(properties[var], "$ref", name + ":" + var)
except:
raise e
Copy link
Contributor

@tchaikov tchaikov Sep 28, 2024

Choose a reason for hiding this comment

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

if $ref is the notation supported by swagger spec, probably we should check if first. and i'd suggest just use dict.get() . simpler this way. and we might want to define a tiny helper to avoid the repeating. like:

def get_referenced_type(schema, key)
    # to be backward compatible, support "type" as well
    type = schema.get("$ref") or schema.get("type")
    if type is None:
       raise KeyError("$ref")
    return type

@tchaikov
Copy link
Contributor

tchaikov commented Sep 28, 2024

also, would be better to use the imperative mood in the title of commit message, in this case, could use something like:

seastar-json2code: allow referencing custom types using "$ref"

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

Successfully merging this pull request may close these issues.

2 participants