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

Add structured fields primitive types to format registry #3390

Merged
merged 4 commits into from
Feb 29, 2024

Conversation

mikekistler
Copy link
Contributor

This PR adds six new formats to the format registry for the primitive types defined in [RFC 8641] Structured Field Values for HTTP.

@mikekistler mikekistler changed the title Add structure fields primitive types to format registry Add structured fields primitive types to format registry Oct 5, 2023
Copy link
Contributor Author

@mikekistler mikekistler left a comment

Choose a reason for hiding this comment

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

De-emphasize the ABNF.

registries/_format/sf-binary.md Outdated Show resolved Hide resolved
registries/_format/sf-binary.md Outdated Show resolved Hide resolved
registries/_format/sf-boolean.md Outdated Show resolved Hide resolved
registries/_format/sf-boolean.md Outdated Show resolved Hide resolved
registries/_format/sf-decimal.md Outdated Show resolved Hide resolved
registries/_format/sf-integer.md Outdated Show resolved Hide resolved
registries/_format/sf-string.md Outdated Show resolved Hide resolved
registries/_format/sf-string.md Outdated Show resolved Hide resolved
registries/_format/sf-token.md Outdated Show resolved Hide resolved
registries/_format/sf-token.md Outdated Show resolved Hide resolved
@darrelmiller
Copy link
Member

darrelmiller commented Oct 5, 2023

@mikekistler These look good but we should also talk about what to do with sf-list and sf-dictionary and more complex headers. /cc @handrews

@handrews
Copy link
Member

handrews commented Oct 5, 2023

@mikekistler I'd like to propose an alternative that encompasses sf-list and sf-dictionary with a different approach. I should have the proposal up by next week's TSC meeting, although probably not in the next few days.

@handrews
Copy link
Member

@mikekistler My apologies for the extremely long delay while I dealt with health issues (and end-of-year holidays).

My concern here is that we have a smooth path to full RFC 8941 in OAS 3.2, as suggested by #1980. I have some thoughts on what that might look like, although I'm still working out the details. I will add them to #1980 when I get it sorted out. Now that I'm able to work properly again, and have mostly caught up on my backlog, I expect to finish that proposal in the next week.

If these are intended only for use with headers with single-item values, I might be OK with them. There will need to be some wording around how you can't just say type: array, items: {format: sf-integer} and have that work on a List-valued structured field. Because my concern with these is that people will think they can automatically use JSON Schema object/list structures with header values that are Structured Field Lists or Dictionaries, and that is not going to be the case.

@mikekistler
Copy link
Contributor Author

@handrews My hope was to have a solution for primitive types in headers -- particularly strings -- without creating any impediments to a solution for full RFC 8941 support.

Are there things in the current PR that would be an impediment to the full solution?

@handrews
Copy link
Member

handrews commented Jan 26, 2024

@mikekistler I think I'm mostly worried about other people running wild with these, doing something unexpected, and then we get stuck having to either support that (because it's "in the wild") or annoy people by breaking something that they think ought to work.

We could just add another sentence to these:

The {{page.slug}} format represents a structured fields byte sequence as defined in [RFC 8941].

along the lines of "This format is currently only intended for use describing single-Item headers; any other usage may not be compatible with future full support for structured fields."

(replacing "single-Item headers" with whatever terminology RFC 8941 mandates).

That way we set boundaries around this and they don't become general-purpose formats that people start using because they're close enough to some other use case they want. Or at least they know not to expect that to work in the long run.

Does that sound reasonable?

@handrews
Copy link
Member

@mikekistler ping! Do you have thoughts on the proposed resolution in my last comment?

@mikekistler
Copy link
Contributor Author

I'm struggling with this because it's just not clear to me that it's necessary. We don't have language like this in any of the other format registry entries.

But for the sake of moving this forward, let me propose a slightly different wording that I think I could live with:

This format is appropriate for a header value that must conform to the {{page.slug}} structured field definition.

Can we compromise on this language?

@handrews
Copy link
Member

@mikekistler Yes, all I'm trying to do is ensure it's used only for the appropriate kind of structured header and not treated as a general-purpose format, and your wording does that. I wouldn't even call it a compromise- I don't much care about the wording, just the results.

@mikekistler
Copy link
Contributor Author

Excellent! I'll make the updates and also fix the RFC references and I think that will address all the outstanding concerns.

@mikekistler
Copy link
Contributor Author

I think all PR comments are addressed. Tagging reviewers for re-review.

Copy link
Member

@handrews handrews left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

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

Thank you @mikekistler and @handrews for getting this done!

@lornajane lornajane merged commit 11d0ae5 into OAI:gh-pages Feb 29, 2024
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.

5 participants