-
Notifications
You must be signed in to change notification settings - Fork 7
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
code optimisation for circular ref issue #62
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SB-rohitdesai, it looks good! Do you plan to add any tests for this functionality though?
Actually everything is already covered to test async await functionality. |
Pull Request Test Coverage Report for Build 9494945514Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes looks good to me. Though few comments are added
Pull Request Test Coverage Report for Build 9498353515Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All those functions look synchronous to me 🤔 Why did you make them async? Can we get some benchmarking results on how it improves the derefencing speed?
Hi @rainum |
Hi @rainum May be because of async its faster , But its giving response quickly than previous code |
We can't solve issues by using an "imperial" approach. There should be a reason for this. I don't want to have so kind of "magical" code in our codebase. @brendarearden, do you have any thoughts regarding this one? |
As far as I know making a function async shouldn't do anything to the execution time of the individual function and it might slightly increase the time because it has to create a promise in addition to running the function. So there is definitely a different underlying reason as to why this change optimized the execution time and I agree that we should have an answer to that. Is it possible that an await was not added somewhere so we just never wait for it to execute? I feel like the linter would've caught that. |
I'd recommend using yalc or something similar to pull these changes into Prism, create a draft PR with the yalced version and add tests in the prism draft pr to demonstrate the optimization if you are not able to add test directly in here. |
Mock server not working In case of circular references.
Motivation and Context
Description
Getting 500 Error or Sometimes might need to wait a long time before getting this error when spec having circular references
Types of changes
Checklist