Conversation
Eventually we could add HTTP resources at this level but HTTP operations doesn't look useful
- add import model and resolver to load consumes adapters from external files - resolve imported consumes entries during capability initialization - support standalone root-level consumes documents in spec model - add custom consumes deserialization for inline vs imported entries - add unit and integration tests for deserialization, resolver behavior, aliases, and error cases - add tutorial step 7 example for consumes import and shared consumes file - update tutorial schema headers to use unified Naftiko schema
|
@eskenazit I've added the implementation to this PR to streamline the review process. I have only left out the "operations" surcharge feature for now. |
| inputParameters: | ||
| - name: "name" | ||
| in: "query" | ||
| value: "${name}" |
There was a problem hiding this comment.
I am not sure there should be a value here since the parameter is in the query. Plus syntax is not mustache ^^'
There was a problem hiding this comment.
This is broken indeed, fixed. This was a draft tutorial proposal that got generated. Feel free with @jeremnaf to take over and reset to something better
src/main/resources/schemas/tutorial/shared-hello-api.consumes.yml
Outdated
Show resolved
Hide resolved
| // Should not throw | ||
| resolver.resolveImports(null, tempDir.toString()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Should we add a test with a import namespace collision to ensure there is a detection and proper error management ?
|
@jlouvel since this is my own PR, I cannot approve or request for changes for your submissions. Let us discuss this in the retrospective to have a proper process in this case =) |
- add import model and resolver to load consumes adapters from external files - resolve imported consumes entries during capability initialization - support standalone root-level consumes documents in spec model - add custom consumes deserialization for inline vs imported entries - add unit and integration tests for deserialization, resolver behavior, aliases, and error cases - add tutorial step 7 example for consumes import and shared consumes file - update tutorial schema headers to use unified Naftiko schema
…/framework into feat/add-import-consumes
jlouvel
left a comment
jlouvel
left a comment
There was a problem hiding this comment.
@eskenazit Argh, I didn't anticipate that.. I have rebased and fixed issues. Should I approve?
| inputParameters: | ||
| - name: "name" | ||
| in: "query" | ||
| value: "${name}" |
| inputParameters: | ||
| - name: "name" | ||
| in: "query" | ||
| value: "${name}" |
There was a problem hiding this comment.
This is broken indeed, fixed. This was a draft tutorial proposal that got generated. Feel free with @jeremnaf to take over and reset to something better
src/main/resources/schemas/tutorial/shared-hello-api.consumes.yml
Outdated
Show resolved
Hide resolved
eskenazit
dismissed
jlouvel’s stale review
The merge-base changed after approval.
|
Do you want me to approve and merge this PR @eskenazit @jlouvel ? |
|
@jeremnaf when it's ready yes, but it is not yet ^^' |
| with: | ||
| name: "Scott" | ||
| outputParameters: | ||
| - type: "string" |
There was a problem hiding this comment.
@jlouve Either missing a name or wrong type
There was a problem hiding this comment.
@jlouvel this whole yml seems very suspicous (no ", strange templating). Was is tested ?
There was a problem hiding this comment.
@eskenazit as I mentioned earlier this is a draft to bootstrap this upcoming step, but there is no "https://api.example.com" API live so it doesn't work. If this is counterproductive, I can just remove this. Let's not waste too much time on this
There was a problem hiding this comment.
Ah I see you are in the Skill adapter example. It is a fake example as well, refers to imaginary API. It is only formally valid and help illustrate. If this is counterproductive, we can remove
|
@eskenazit I've removed the tutorial step 7 and associated YAML file. This is delayed this PR and has little value at this point |
3 participants
added consumes import support and made some chore cleaning