search

instructlab / dev-docs

Developer documents for the InstructLab organization
Apache License 2.0
2 stars 27 forks source link

Api definition guidelines #71

Open gabe-l-hart opened 4 months ago

gabe-l-hart commented 4 months ago

Description

This PR introduces a framework for managing OpenAPI definitions for InstructLab APIs and Platform APIs.

russellb commented 4 months ago

This looks pretty good to me, thanks!

My main request is that we merge #69 first (should be able to next week) and then we treat this as one of the first artifacts coming out of that working group.

The only change I'd suggest then is to move the doc/api-definition-guidelines.md file under docs/backend/, but that requires #69 to be in first.

There is a bit of additional context I'd like to capture, but I'm OK not blocking this PR on it. For example, this is written with an architecture in mind about an InstrutLab API and supporting platform APIs. I'd like to write a high level backend direction doc that would introduce these concepts. I have existing content for this. If feedback on that doc changes anything substantially, we can always adjust these docs too. It's all a moving target.

russellb commented 4 months ago

The hold label is just for waiting on #69.

gabe-l-hart commented 3 months ago

That sounds good. I have an internal platform ADR about the decision to make REST APIs as the common service layer that we can adapt for the general backend architecture perspective.

russellb commented 3 months ago

69 merged, so I'll drop the hold

relyt0925 commented 3 months ago

This looks good to me!

booxter commented 3 months ago

Directionally, this looks fine. The initial guidelines are quite abstract and perhaps will be clarified / filled with essence once actual APIs are proposed and actual bindings are generated. The main point is - the guidelines clarified that bindings will be auto-generated and hosted elsewhere; and there will be a separate repo for schema itself. This should be enough to merge it IMO.

hickeyma commented 2 months ago

@markstur @russellb The guideline looks good to go. Do you have any further comments or obejctions to merging and moving forward?