Closed glodieu closed 8 years ago
We're going to update the Travis node version so that tests run (@rainum).
Everything looks good, but we'd like to continue supporting RAML 0.8 export. Moving to 1.0 only export would be a breaking change for many of our users unfortunately.
At the moment, we don't have a good way to support multiple spec versions in api-spec-converter. The two options I see are:
1) duplicate the entire RAML exporter, one for 0.8, and one for 1.0. 2) pass the version number into exporters, and then have the one raml exporter use that version number to tweak the export as appropriate.
I vote for #2. However, it's a bandaid - we are currently preparing an architecture document for v2 of api-spec-converter, and it will handle multiple versions in a much more robust way.
Open to suggestions!
How about providing different exporter for a specific version? For that you don't need to do any changes to the current, but that would mean you need to maintain two different versions and there might be many similarities.
Yup that is option #1 above - we could certainly do that as a temporary solution, in order to get RAML 1.0 support out the door. If there are not many differences between the two versions, it might make sense to do option #2 above though - one exporter that is passed the version number.
@rainum and I are putting together an architecture document for a a significant update to this converter, which we'll share later this week. This will include a longer term solution to the spec versioning issue.
@glodieu @marbemac did we agree on a method to support both versions of RAML instead of replacing one?
@sichvoge Yes. Do you think that I need to create a new issue "support raml 0.8 and 1.0" or use this one to maintain raml 0.8?
Can you quickly summarize the outcome. What are we going for? Option 1 or 2?
I don't think you need to create another issues, because RAML 0.8 is already there; only update the initial issue for adding RAML 1.0 support about how thats gonna been done.
Hi @sichvoge @marbemac We're going with option #2 but if we found many differences we'll change to option #1.
@glodieu That sounds great, let me know if there is anything that we can do to help, or if you have questions! Looking forward to shipping this.
Hi Marc, already finish. I created a baseraml,js with all common methods, raml08.js and raml10.js for the things regarding each version. Thanks!
Awesome! Vazha is on vacation this week, so it might be a few days delays, but we'll take a look at this and merge in asap. Thank you for the work, it's very much appreciated!
@rainum to look at on Monday.
We're adding support for multiple content types to Stoplight, and have updated this library accordingly to take advantage of that. It's a work in progress over at https://github.com/stoplightio/api-spec-converter/tree/feature/mimetype-overhaul. We'll merge these changes into that branch, test everything, and then finally merge back into master.
Question: does RAML 1.0 support multiple mimetypes? If so, we'll update the exporter to take advantage of this multiple mimetype support as well.
@marbemac it does support the use of multiple mimetypes indeed. See here.
@marbemac exporter is already updated to support multiple mimetypes for RAML 1.0
@sichvoge @glodieu Our build tools are going crazy with the addition of the raml1 parser package (basically not working). We just did an experiment adding webpack, to pre-compile a bundle.js of api-spec-converter and distribute it with the repository.
Without the raml1 package, bundle.js is 2.88mb. With the raml1 package, bundle.js is 20mb (!!!).
Do you guys have any idea what's going on?
Ah, just noticed these, which might be related:
https://github.com/raml-org/raml-js-parser-2/issues/361 https://github.com/raml-org/raml-js-parser-2/issues/432
I suspect something to do with the raml1 parser dependencies is causing the build tools we use for the Stoplight app to die when trying to compile everything.
I removed raml 1.0 support from the feature/mimetype-overhaul for right now.
I created feature/raml10 branch with raml1.0 support.
@marbemac thats a known problem we need to fix indeed. the size of the project is to big and we will try to make some improvements soon.
@glodieu @sichvoge any ideas on the following error? This occurs when you try to build, with webpack, a project importing the raml1 package (when you include building the node_modules deps).
You can reproduce the issue by checking out the https://github.com/stoplightio/api-spec-converter/tree/babel-build-fail branch, running "npm i" and "npm run build". The webpack config used is in webpack.config.js.
Unfortunately this issue means we can't compile the stoplight codebase. We've spent quite a bit of time on it, and for now will have to just push the mimetype updates to master without raml 1.0 just yet. We do badly want to support 1.0 though!
> webpack --config ./webpack.config.js --progress --colors
63% 366/409 build modules
<--- Last few GCs --->
28568 ms: Mark-sweep 1396.6 (1456.5) -> 1396.6 (1456.5) MB, 853.4 / 0 ms [allocation failure] [GC in old space requested].
29410 ms: Mark-sweep 1396.6 (1456.5) -> 1396.4 (1456.5) MB, 842.5 / 0 ms [allocation failure] [GC in old space requested].
30234 ms: Mark-sweep 1396.4 (1456.5) -> 1396.4 (1456.5) MB, 824.0 / 0 ms [last resort gc].
31071 ms: Mark-sweep 1396.4 (1456.5) -> 1396.2 (1456.5) MB, 836.6 / 0 ms [last resort gc].
<--- JS stacktrace --->
==== JS stack trace =========================================
Security context: 0x1ddd2f1e3ac1 <JS Object>
1: parseLiteral [/Users/Marc/Sites/stoplight/yogi/node_modules/babylon/lib/parser/expression.js:~545] [pc=0x2e20ef9ee8e3] (this=0x123ad2dbd961 <a Parser with map 0x143363facd81>,value=0,type=0x3ef0bad3469 <String[14]: NumericLiteral>)
2: parseExprAtom [/Users/Marc/Sites/stoplight/yogi/node_modules/babylon/lib/parser/expression.js:~387] [pc=0x2e20ef9babe6] (this=0x123ad2dbd961 <a Parser ...
FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - process out of memory
[1] 6094 abort npm run build
@marbemac we will try to help investigating. haven't seen that before and looking around it might be typescript that cause issues, but i don't know.
Seems the problem is the typescript dependency from the parser. As the team is cleaning up legacy dependency, that typescript dep will be removed as well. Hope that will simplify the packaging significantly. We will probably publish a version tomorrow.
With the new raml1 parser update, we can now compile! Will be working towards getting this in shape in into production asap. A quick check of exporting our todo demo yields some errors in api workbench that we have questions about. Otherwise, it's looking good.
#%RAML 1.0
title: To-do Demo
version: '1.0'
baseUri: 'http://todos.stoplight.io'
protocols:
- HTTP
documentation:
- title: To-do Demo
content: |-
## Welcome
This is a place to put general notes and extra information, for internal use.
To get started designing/documenting this API, select a version on the left.
securitySchemes:
'null':
type: Pass Through
describedBy: null
description: null
/todos:
displayName: todos
description: ''
'/{todoId}':
displayName: '{todoId}'
description: ''
uriParameters:
todoId:
type: string
get:
displayName: GET_todo
headers: {}
responses:
'200':
body:
application/json:
$ref: '#/definitions/todo-full'
queryParameters: {}
put:
displayName: PUT_todos
body:
application/json:
$ref: '#/definitions/todo-partial'
example: |-
{
"name": "my todo's new name",
"completed": false
}
headers: {}
responses:
'200':
body:
application/json:
$ref: '#/definitions/todo-full'
'403':
body:
application/json:
type: object
queryParameters: {}
securedBy:
- null
delete:
displayName: DELETE_todo
headers: {}
responses:
'204': {}
queryParameters: {}
securedBy:
- null
uriParameters: {}
post:
displayName: POST_todos
body:
application/json:
$ref: '#/definitions/todo-partial'
example: |-
{
"name": "my todo's name",
"completed": false
}
headers: {}
responses:
'201':
body:
application/json:
$ref: '#/definitions/todo-full'
'403':
body:
application/json:
type: object
properties: {}
queryParameters: {}
securedBy:
- null
get:
displayName: GET_todos
headers: {}
responses:
'200':
body:
application/json:
type: array
items:
$ref: '#/definitions/todo-full'
example: |-
[
{
"id": 17,
"name": "new lit item",
"completed": null,
"completed_at": null,
"created_at": "2014-08-28T14:14:28.494Z",
"updated_at": "2014-08-28T14:14:28.494Z"
},
{
"id": 18,
"name": "new lit item2",
"completed": null,
"completed_at": null,
"created_at": "2014-08-28T14:14:28.494Z",
"updated_at": "2014-08-28T14:14:28.494Z"
}
]
queryParameters: {}
/mocked:
displayName: mocked
description: ''
uriParameters: {}
get:
displayName: GET_mocked
description: |-
This is an example of a mocked endpoint.
This endpoint does not actually exist in the api - try visiting [http://todos.stoplight.io/mocked](http://todos.stoplight.io). You will see the default response (same as you get when you visit the root "/" url).
We have defined a 200 response below, with an example, and then explicitly turned on mocking for this endpoint via the dropdown in the right sidebar.
With this enabled, if you visit {your prism instance host}/mocked, you'll see the mocked example. You can find the mock server host for this API by clicking the "Design Dashboard" button at the top of this screen. You can also try sending a test request by clicking "send test request" button in the right sidebar.
headers: {}
responses:
'200':
body:
application/json:
type: object
properties:
this:
type: string
mocked:
type: boolean
example: |-
{
"this": "is",
"mocked": true
}
queryParameters: {}
types:
user: |-
{
"type": "object",
"properties": {
"name": {
"type": "string"
}
}
}
todo-partial: |-
{
"type": "object",
"properties": {
"name": {
"type": "string"
},
"completed": {
"type": [
"boolean",
"null"
]
}
},
"required": [
"name"
]
}
todo-full: |-
{
"allOf": [
{
"$ref": "#/definitions/todo-partial"
},
{
"type": "object",
"properties": {
"id": {
"type": "integer"
},
"completed_at": {
"type": [
"string",
"null"
],
"format": "date-time"
},
"created_at": {
"type": "string",
"format": "date-time"
},
"updated_at": {
"type": "string",
"format": "date-time"
}
},
"required": [
"id"
]
}
]
}
traits:
- standardErrors:
responses:
'401':
body:
application/json:
type: object
'404':
body:
application/json:
type: object
properties:
foo:
type: string
bar:
type: string
- standardErrors:
responses:
'403':
body:
application/json:
type: object
properties:
message:
type: string
required:
- message
example: |-
{
"message": "NOT AUTHORIZED"
}
'404':
body:
application/json:
type: object
properties:
status:
type: string
error:
type: string
required:
- status
- error
example: |-
{
"status": "404",
"error": "Not Found"
}
'500':
body:
application/json:
type: object
- paged:
queryParameters:
limit:
type: integer
description: This is how it works.
maximum: 100
skip:
type: string
Hi Marc, Could you please share me source file used to generate such raml file? Thanks!
References are handled directly. So lets assume you have:
type: object
required:
- name
properties:
name:
type: string
address:
$ref: '#/definitions/Address'
age:
type: integer
format: int32
minimum: 0
that converts into:
type: object
properties:
name:
type: string
address:
type: Address
required: false
age:
type: integer
format: int32
minimum: 0
required: false
We discussed some of the possible conversions. @glodieu would you be able to share the document we have with Mark.
Hmm, so are $refs not supported? It's a common pattern to make the body definition a ref to a model. For example, /users returns an array of user models, /user/{id} a single user model, etc. Those have $ref right there in the root (like in the base below).
post:
displayName: POST_todos
body:
application/json:
$ref: '#/definitions/todo-partial'
This in RAML is:
post:
displayName: POST_todos
body:
application/json:
type: todo-partial
- Seems we need to update traits to be a map in the 1.0 exporter (instead of an array).
yes it needs to be a map. what i also realised is that there are a lot of empty nodes, we should check if a node is empty and if yes not create it.
AH, ok - so we just need to add support for re-writing those root $refs to that format.
I'm ok with the empty node change.
Any idea on the example property in the image above?
Awesome, thanks for this guys! We'll take a look at it all on Monday. @rainum to comb through it, and figure out what's happening w Travis.