-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
fix: generating fromTemplate with refs in spec #945
fix: generating fromTemplate with refs in spec #945
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.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
d2ec6e4
to
912b417
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@Laupetin hey, thanks for the PR. I just had a look at related issue and started digging a bit as it was confusing why it works in Generator CLI (next time please specify version that you use, it is super important) and do not work in AsyncAPI CLI. Please have a look at asyncapi/parser-js#797 (comment) |
@derberg Hi, thanks for checking it out. Sorry for the delayed answer. I can reproduce this issue with From my understanding the root of this issue is that the CLI first loads the spec from the file to a string in-memory. The parser therefore does not know the directory in which the spec is located in. What does work with the CLI is having an absolute path inside the root spec file. # index.yaml
asyncapi: 2.5.0
info:
title: Example
version: 0.1.0
channels:
user/signedup:
$ref: /home/user/projects/sample/subfolder/user-signedup.yaml ^ This would work. It is not very practical however when having the spec in a git repository that could be checked out with different paths as well. I could not manage to get anything working with the fix inside the linked comment. |
912b417
to
f5247f3
Compare
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
Quality Gate passedIssues Measures |
/update |
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.
I've been trying to reproduce the bug related #944 and seems to be solved (output is correctly generated) working under the current CLI version 1.14.1
👍
@Laupetin Could you confirm you are not experiencing the issue anymore? In that case we may proceed to close this PR and also the bug related. Thanks! @Souvikns @Shurtu-gal @Amzani
@peter-rr hey, thanks for checking it out. You are right, the exact case described in the issue seems to now generate correct output. If the paths instead are |
f5d4c7c
to
d081928
Compare
I rebased the branch on the current master branch. The following scenario does not work with the master branch but does with the changes of this PR (the # docs/index.yaml
asyncapi: 2.5.0
info:
title: Example
version: 0.1.0
channels:
user/signedup:
$ref: subfolder/user-signedup.yaml # docs/subfolder/user-signedup.yaml
subscribe:
message:
description: An event describing that a user just signed up.
payload:
type: object
additionalProperties: false
properties:
fullName:
type: string
email:
type: string
format: email
age:
type: integer
minimum: 18 |
/u |
/update |
/update |
Quality Gate passedIssues Measures |
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.
LGTM 🚀
Everything working fine with fix applied.
/ptal |
@Amzani @derberg @magicmatatjahu @Souvikns @Shurtu-gal Please take a look at this PR. Thanks! 👋 |
/update |
Quality Gate passedIssues Measures |
/u |
/u |
@Souvikns @Shurtu-gal @Amzani FYI: I also opened this issue as a potential improvement to avoid this kind of scenarios: asyncapi/.github#306 |
@peter-rr for now we had to disable sonar cloud - to many problems comparing to the value it brings. hope we can get alternative solution you reported done quickly /rtm |
🎉 This PR is included in version 2.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Updates the code for generation via template to add the path of the specification file to the options passed to the generator.
This makes the generator able to resolve paths specified in
$ref
properties.Related issue(s)
Fixes #944