-
Notifications
You must be signed in to change notification settings - Fork 92
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
Ultra-light version for validation purposes only #68
Comments
Sway's size shouldn't be an issue on the node.js side of things. We've had concerns with Sway's performance before and most of it was because Sway fully resolves a Swagger document so if you have a lot of nested references and circular references and such, you can have a gargantuan document pretty easily. The easiest way to test this:
This will tell you how many lines the fully resolved Swagger document is. If you have a document that can reproduce this, I would love to spend some time doing some performance analysis. I am sure size could have something to do with slowness but I wouldn't expect it in node.js land because none of these larger, slower dependencies are in the chain for validation. I realize this document might be sensitive and if that is the case, I can sign an NDA and we can do this offline or whatever is necessary. I'd love to fix this. |
Thanks @whitlockjc, test.js (without sway) console.time('no-sway');
console.timeEnd('no-sway'); output
test.js (with sway) console.time('sway');
require('sway');
console.timeEnd('sway'); output
Meaning that by just adding the library, without loading any definition, there is a delay of 482ms? |
Any way you can tell me the output of the following:
To run this, This will help me start the process of figuring out what is going on. Behind the scenes, Sway does the following to process a Swagger document:
At one point, I started the process of fixing performance concerns before they were reported. Since you've reported them, I think some of the enhancements I had started but not finished will likely be necessary but I really need to have an idea of what I'm working with. |
Actually what I have mentioned on my last response is what is taking more time in my opinion, not the time for parsing the definition itself, but instead the time it takes just for requiring sway which is approx. half second? So if you imagine on the server where I am validating incoming requests against the definition, half second is spent only requiring sway? the parsing/validation process is actually pretty quick so I think it would be ok in this aspect? |
Oh, gotcha. I didn't notice that. So just a |
Yes! :D Thanks Jeremy. That was my point about Thanks again |
I'll figure it out. |
Thanks Jeremy, I might be able to help since this is something I will be
|
Well, Sway isn't just a validation API. I could break the validation out, and I might end up doing that for https://github.com/whitlockjc/swagger-lint, but
swagger-tools had this feature but it was locked up in the swagger-router middleware and it was a homegrown feature. As for why |
@whitlockjc I also use only Swagger validation functionality in api-spec-converter and this lib is used in a browser. I would suggest creating separate lib just for validation of Swagger(without external |
That's likely what we'll do. Right now I have started a project called swagger-lint and pending a rename, we could move the validation parts of sway into it and use that as the lib for simple validation. I apologize for the longer release cycles but I'm not working much on Swagger tooling at work these days. If anyone has time to help, I'd greatly appreciate it. |
I imagine that In my case, I can't convert spec without resolve external references so the output is always self-contained. If we can extract minimal version of validation library, just validation code without anything else. @whitlockjc What do you think?
This functionality is very important for me. |
So we want another library that is just validation, no loading/reference dereferencing/..., and then sway would use that? In all honesty, the size of Before we create any new repository, please let me check with Apigee first. Sway is under their organization right now and I need to make sure I can break code out and if so, how we need to do it. I'm sure it's fine but it will likely have an impact on where it's created. I'll see if I can get sway and this new repository moved to |
You right So I need to integrate both validators into my CI and, more importantly, keep them in sync with each other. I have nightmares about some 3rd validator became popular :( IMHO, I think the simple library is the more adoption it will get as part of 3rd-party party libs/tools.
Any account will work for me.
@zanona What do you think? |
I still don't know why In the end, I just wanted to provide a useful but simple object model and API around Swagger and its validation. Anything you think we can do to make that better is well received. |
I can't agree more but only if work with files which can contain external refs. But if you know for sure that you don't have any external refs at all, an async interface is unnecessary complicates code. See
I totally agree that it should stay like that. In particular situation that I experienced it was this check:
And I can think of other useful checks for example, check that But this issue is not about that. An important thing is I want to contribute such PRs but preferably only into one tool.
It's real world people always have their own motives. But important things is to find points where you can collaborate with them and create win-win scenarios. |
The missing validations:
|
If you know they're missing, file bugs and we'll get it sorted for sure. |
I believe that current approach is too complicated you need to traverse JSON Schemas with your own code. It's very error-prone and also duplicate things which But this beside the point of this issue, important thing that I want to ensure interoperability for all my clients. That mean I want to contribute my work into a library which is widely used. |
Hi @whitlockjc |
No, I'll look at it this weekend. Day job has kept me pretty busy the past few weeks. |
@whitlockjc I can help. |
I have been playing around with swagger-js and I thought that the 1.5 MB uncrompressed was large, but loading 1.1 Mb compressed in a (mobile) browser to be able to call a swagger service is way to much. Having a swiss army knife with 50 tricks is nice, but when you need to peel potato's its not really convenient. If sway is to be the official JS library then something more modular would be much appreciated. A number of features, like mock generation, are brilliant to devs, but won't be used in production. So being able to drop that weight would be a big plus imho. Btw: https://github.com/signalfx/swagger-client-generator does it in 17Kb minified including client side validation, however that only supports swagger 1.2 :-( |
I will be working on this starting today. Once I have the repository setup, I will alert you all and I'll make sure Gitter is enabled so we can collaborate. Thanks for your patience. |
@whitlockjc Thank you for the update. |
@whitlockjc Any progress on this one? |
I started the work yesterday. |
Here is the repo: https://github.com/whitlockjc/oas-validator I've not put anything into it yet but just know I'm working on it. I plan on releasing Sway today, |
|
I'm going to suggest we use #187 to keep track of this for now on. The idea will be to break |
Hey there,
I am currently using sway to validate server requests against swagger definitions which is really great.
However, I have noticed that at the moment sway is quite slow due to the fact it is currently using dependencies such as
json-shcema-faker
having a massive amount of information embedded into the library, while that is great for generating data, it might not be as efficient for those who only need to validate swagger definitions, so I would like to ask if there is any way to reduce sway to an optional lighter version that would provide best performance results?Thank you in advance
The text was updated successfully, but these errors were encountered: