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

parse_openapi_spec() improvements #187

Merged
merged 7 commits into from
Jul 7, 2023
Merged

Conversation

mgirlich
Copy link
Owner

@mgirlich mgirlich commented Jul 5, 2023

Fixes #186.

Now the three specs that failed all work:

asana_yaml <- yaml::read_yaml("https://raw.githubusercontent.com/Asana/openapi/master/defs/asana_oas.yaml", handlers = list(int = as.character))
asana_spec <- parse_openapi_spec(asana_yaml)

zoom_yaml <- yaml::read_yaml("https://api.apis.guru/v2/specs/zoom.us/2.0.0/openapi.yaml", handlers = list(int = as.character))
zoom_spec <- parse_openapi_spec(zoom_yaml)

youtube_yaml <- yaml::read_yaml("https://api.apis.guru/v2/specs/googleapis.com/youtube/v3/openapi.yaml")
youtube_spec <- parse_openapi_spec(youtube_yaml)

While this can now be parsed, the performance could be improved. After some minor changes the performance is quite okay.

@jonthegeek
Copy link

Amazing, thank you! This appears to work great!

How would you feel about including the rest of the information in the spec (perhaps subject to arguments in parse_openapi_spec)? You're parsing the returns, which is definitely useful, but I'd like to also get the arguments, descriptions, etc.

At a minimum I'll be building off what you have (and using tibblify as the format for including specifications about what to expect), but it'd be helpful if it were built in completely.

@mgirlich mgirlich merged commit fb6d4b8 into main Jul 7, 2023
@mgirlich mgirlich deleted the open_api_parser-improvements branch July 7, 2023 09:56
@mgirlich
Copy link
Owner Author

mgirlich commented Jul 7, 2023

I'm not quite sure. It kind of makes sense as this might be quite easy to integrate. On the other hand, this doesn't really fit to tibblify. I'll have a look into this to get a better feeling for the whole issue.

@jonthegeek
Copy link

To be clear, I just want to include everything that fits into that tibble; to add the missing columns from the incoming spec. I'm not sure why that wouldn't fit in tibblify.

@mgirlich
Copy link
Owner Author

mgirlich commented Aug 2, 2023

@jonthegeek I added a PR #191 that parses more fields of the Open API spec. Does that help you or do you already have a parser by now?

@jonthegeek
Copy link

@jonthegeek I added a PR #191 that parses more fields of the Open API spec. Does that help you or do you already have a parser by now?

Good timing! I haven't been focused on this part, but today I'm spending the whole day working out a formal "R API definition" class that will be the direct input for my functions in a couple different packages, so I was going to see if what you do here will still be useful, or if it makes more sense to go from scratch. I suspect I'll want to go from scratch, because I want ALL of the definition, and some things almost definitely won't be tibbles... But I'll know more in a few hours!

@jonthegeek
Copy link

Yeah, this looks great... but I don't think it makes sense for tibblify to cover what I'm looking for. Keep an eye on {rapid} for my R API Definition package, which will HOPEFULLY have a v0.1 in the next couple days (or at least a PR with the basic idea laid out).

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.

parse_openapi_spec vs Asana
2 participants