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

API Reference documentation #24

Merged
merged 2 commits into from
May 6, 2018
Merged

API Reference documentation #24

merged 2 commits into from
May 6, 2018

Conversation

fvanroie
Copy link
Member

@fvanroie fvanroie commented May 5, 2018

By popular request I have documented the API calls for inclusion in the OPNsense documentation. This is the first draft, but should be a good starting point as there is currently no comprehensive list of API calls available.

The list is based on the source code and auto-generated by a script. I have reviewed it and use the same list already in my project. I present this PR as a starting point for discussing the API documentation and I'm open to any comments.

@fichtner fichtner assigned fichtner and AdSchellevis and unassigned fichtner May 6, 2018
@fichtner
Copy link
Member

fichtner commented May 6, 2018

Awesome, thanks for this! ❤️

Handing over to @AdSchellevis for review/merge.

@AdSchellevis
Copy link
Member

@fvanroie do you mind if we split the file and move it around in the menu structure? I think this is a good starting point. I can merge this in a make some suggestions, if you're willing to keep working on it.

@fvanroie
Copy link
Member Author

fvanroie commented May 6, 2018

No problem @AdSchellevis. I figured the How-to's location wasn't the best place for it. As long as the page(s) retains the csv-table data format, I will be able to parse, re-import and update it without any problems.

Ideally the API reference should be updated on new PRs in source, but I know from experience this gets overlooked quite easily. I have some PowerShell scripts that do the heavy lifting already, so it's not much effort on my part. If anyone is interested, I don't mind sharing that too.

@AdSchellevis AdSchellevis merged commit fa2f7fd into opnsense:master May 6, 2018
@AdSchellevis
Copy link
Member

@fvanroie I've moved all to a top level section "API reference" in the development area (d9f2b20)

I don't use PowerShell myself, but am certainly interested in the scripting, maybe we can rewrite it at some point to python to fit it in our build tools.

Shall I publish what we have now? Maybe that inspires others to write some additional documentation around the described endpoints. (If I understand your work sofar, your scripting replaces the table sections and leaves the rest of the content intact).

@fvanroie
Copy link
Member Author

fvanroie commented May 6, 2018

I pulled the changes down and rendered it in Sphinx and that looks nice :-) I think you can publish it like it is and we can go from there.

For the moment I only have a generator script. Now that a final file structure is in place, I can rework it as an updater script. It can then check if an API call is in the documentation or not and add it if necessary...

@AdSchellevis
Copy link
Member

let's do that, nice progress this. https://docs.opnsense.org/development/api.html

@fichtner
Copy link
Member

fichtner commented May 6, 2018

love it, thanks to you both! :)

@fabianfrz
Copy link
Member

@fvanroie If you can look into the function body and find a single call to a method with base in the name, the call is for interaction with the model. You would be able to add the API result as well because you can read the data format from the model.

@fvanroie
Copy link
Member Author

fvanroie commented May 6, 2018

@fabianfrz I'll have a look at that, allthough I already have a function that parses the model.xml files to create classes for CRUD items. So getting the info is not really a problem, but presenting it in a concise manner using RST is more of a challenge for me.

Does RST have a collapsable codeblock option? That way this info could be included whithout causing too much clutter on the page.

@fabianfrz
Copy link
Member

fabianfrz commented May 7, 2018

@fvanroie sorry, I don't know that, but it is the job of the renderer anyway (maybe the template needs to be adopted to do that).

Collapse does not work with PDF so it probably cannot be in the table anyway because the HTML has limited width too.

@fvanroie
Copy link
Member Author

@AdSchellevis I reworked the API extraction script in Python here fvanroie/opnsense_core@24d8644. Most of it is git, grep and regex to pull the data in.

@AdSchellevis
Copy link
Member

@fvanroie cool! Are you planning to update the doc pages with this script at some point too?

@fvanroie
Copy link
Member Author

fvanroie commented May 11, 2018

@AdSchellevis Yes, that's still the plan. I'm just looking for the right tool for the job. My PowerShell is better than my Python skills. In the meantime I have found a nice API reference written in Sphinx that looks great. I'll look into those RST pages for some stlying guidelines.

I've also been looking at the OpenApi documentation tools provided by Swagger. I adjusted the output to produce OAS code instead of RST, including the data models for CRUD items. A preview can be found here.

@AdSchellevis
Copy link
Member

@fvanroie I've tried slate some time ago with another project, which was quite practical, but gathering all in Sphinx into the same documentation base certainly has it advantages. If you help on the python part, just let me know. It would be cool if we could generate the basics and annotate the details in these documents.

@fvanroie
Copy link
Member Author

fvanroie commented May 19, 2018

I published the initial opnsense_MakeDocs PowerShell Module on github. It's the build script from source 2 rST and works on Windows and Linux. There is a dependency on some handcrafted json files (included in the module) for now... I'm in favor of removing this dependancy for of an OpenAPI specification file as the single source of thruth. That format seems to be the de-facto standard for APIs and widely supported...

On that note, I also built my first OPNsense plugin called SwaggerUI. It's an interactive API Reference viewer built on top of Swagger-UI-dist. The viewer can be installed in OPNsense, kind of what @juggie suggested in opnsense/core#1646.

The proof-of-concept:
swaggerui

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

Successfully merging this pull request may close these issues.

None yet

4 participants