Open mfroberg opened 8 months ago
Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup 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.
I know this is not too helpful, but wanted to share that I am observing the same problem here as well, using asyncapi/cli:1.8.4
. The referenced files remain referenced, and not bundled up.
@mfroberg @francocm Thanks for reporting, I'm having a look at this 👀
@Amzani and @peter-rr I've raised a draft PR that replicates this problem: https://github.com/asyncapi/cli/pull/1389
I can try giving a look later at the root cause too, but in the meantime I hope that's helpful.
Also, existing assertions that validated the output spec were not actually asserting for to.be.true
.
@Souvikns @Shurtu-gal The issue occurs just with v3 documents. I've been doing some research but haven't managed to find the root cause. Would you have any clue about it? 🤔
@peter-rr I've updated the related PR slightly regarding the way assertions are done, and also which referenced files etc are used for comparison.
I spent some time trying to get to the root cause, but I am suspecting it's originating from https://github.com/asyncapi/bundler. This repo uses 0.4.0
. I noticed that 0.5.0
is available, but the upgrade to 0.5.0
is not straightforward. A lot of tests started failing after upgrading, as well as I noticed that 0.5.0
changed the signature and configuration options.
So this requires deeper investigation and also a plan for upgrade.
Hey @francocm , thanks for the updates on the PR 🙌 I'm having a look at them!
Yeah, the root cause must be definitely at the @asyncapi/bundler
repo. Regarding the latest available version 0.5.0
, it should be correctly updated at CLI repo since last week but there might be an issue related to the bot in charge of bumping to latest versions. Here is the info I have about it so you can take a look 👀 @Souvikns @Shurtu-gal @Amzani
I can also create a PR to update the version manually in case the auto-update is not working properly.
Hey @peter-rr I am the one who reverted it. The bundler has to be migrated separately as there were some changes made in 0.5.0
due to which the bundle.command was not working properly.
Ref: #1372
Cool @Shurtu-gal 👍 Thanks for the info! Let me know if you need some help in order to solve the issue :)
I have pulled from upstream into https://github.com/francocm/asyncapi-cli/tree/fix/1323/bundleNotBundlingExternalFiles
and went from 2 failing tests under test/integration/bundle/bundle.test.ts
to 20, with the errors now including:
20) validate
with no context file
throws error message if no context file exists:
AssertionError: expected 'ContextError: No context is set as cu…' to equal 'error locating AsyncAPI document: The…'
+ expected - actual
-ContextError: No context is set as current, please set a current context.
+error locating AsyncAPI document: These are your options to specify in the CLI what AsyncAPI file should be used:
+ - You can provide a path to the AsyncAPI file: asyncapi <command> path/to/file/asyncapi.yml
+ - You can provide URL to the AsyncAPI file: asyncapi <command> https://example.com/path/to/file/asyncapi.yml
+ - You can also pass a saved context that points to your AsyncAPI file: asyncapi <command> context-name
+ - In case you did not specify a context that you want to use, the CLI checks if there is a default context and uses it. To set default context run: asyncapi config context use mycontext
+ - In case you did not provide any reference to AsyncAPI file and there is no default context, the CLI detects if in your current working directory you have files like asyncapi.json, asyncapi.yaml, asyncapi.yml. Just rename your file accordingly.
+
at Context.<anonymous> (test/integration/validate.test.ts:164:31)
at Object.run (node_modules/fancy-test/lib/base.js:44:38)
at Context.run (node_modules/fancy-test/lib/base.js:68:36)
It looks like this is a result of some error messaging/handling changes.
Nonetheless, with the state of 'broken' tests currently in master
, these were not detected. I can refactor the tests to align with the updated output, but:
test/integration/bundle/bundle.test.ts
captures.I can't spend too much time on it right now, but happy to do any small changes in my branch if you wish.
Any thoughts @peter-rr @Shurtu-gal
Thanks @aeworxet this issue seems to be resolved through your change.
I've noticed a new potential gap, which I've detailed in a comment in my tests PR here: https://github.com/asyncapi/cli/pull/1389#issuecomment-2102950792
@francocm @mfroberg
After the @asyncapi/bundler
dependency upgrade, the file we are getting when running asyncapi bundle main.yaml
looks like:
asyncapi: 3.0.0
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
userSignedUp:
address: user/signedup
messages:
UserSignedUp:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the user
operations:
onUserSignUp:
action: receive
channel:
$ref: '#/channels/userSignedUp'
messages:
- $ref: '#/channels/userSignedUp/messages/UserSignedUp'
components:
messages:
UserSignedUp:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the user
Could you confirm this is the expected bundled file? I'm not sure since the messages.yaml
content is shown duplicated, meaning the referenced object has been dereferenced twice after bundle process 🤔
@peter-rr
Hey. Me again. :wave:
I modified Optimizer
(v1.0.0+
) some time ago (simultaneously with Bundler
v0.5.0
) exactly for these cases.
After Optimizer
v1.0.2
(current master
) with
{
output: 'YAML',
rules: {
reuseComponents: true,
removeComponents: true,
moveAllToComponents: true,
moveDuplicatesToComponents: false,
},
disableOptimizationFor: {
schema: true,
},
}
the output file looks like this:
asyncapi: 3.0.0
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
userSignedUp:
$ref: '#/components/channels/userSignedUp'
operations:
onUserSignUp:
$ref: '#/components/operations/onUserSignUp'
components:
messages:
UserSignedUp:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the user
operations:
onUserSignUp:
action: receive
channel:
$ref: '#/channels/userSignedUp'
messages:
- $ref: '#/channels/userSignedUp/messages/UserSignedUp'
channels:
userSignedUp:
address: user/signedup
messages:
UserSignedUp:
$ref: '#/components/messages/UserSignedUp'
Does this suit your needs better?
@KhudaDad414
How would the above option object look in the form of a command for asyncapi optimize
? I haven't delved into that code yet.
@aeworxet currently there is no way to ignore schemas in CLI. I have opened an issue here: https://github.com/asyncapi/cli/issues/1432
btw, I don't think it's a good solution for the issue that @peter-rr reported. https://github.com/asyncapi/cli/issues/1323#issuecomment-2115681856
The problem is that the bundler de-references the local reference in channels/userSignedUp/messages/UserSignedUp
. not sure If this is an expected behaviour.
@KhudaDad414
The problem is that the bundler de-references the local reference in
channels/userSignedUp/messages/UserSignedUp
. not sure If this is an expected behaviour.
I was going from the v3.0.0 Spec: channels/channel/messages/message
is supposed to be dereferenced.
Bundler
does this and $ref: '#/channels/userSignedUp/messages/UserSignedUp'
is pointing to the correct location at that time.
Then Optimizer
moves channels/userSignedUp
to components
but doesn't add components
to the $ref
, which now should be $ref: '#/components/channels/userSignedUp/messages/UserSignedUp'
.
What else should be changed after this issue is fixed?
currently there is no way to ignore schemas in CLI.
Maybe format of
disableOptimizationFor: {
schema: true,
},
should be rethought in a way that makes both options (in standalone and CLI) consistent?
Like
{ disableOptimizationForSchema: true } // in the standalone Optimizer
--disableOptimizationForSchema // in CLI's command line, as the very presence of the cmd switch already
// means 'true' and its absence - 'false'
{ disableOptimizationForSchema: flags.disableOptimizationForSchema } // in CLI's 'optimize.ts'
?
The issue with updating the $ref
s of the AsyncAPI Document after moving the AsyncAPI components to the components
section by prefixing the corresponding JSON Pointers with the characters components/
will be solved by https://github.com/asyncapi/optimizer/pull/263.
I was going from the v3.0.0 Spec: channels/channel/messages/message is supposed to be dereferenced.
@aeworxet if we leave the optimizer out of the picture for now, which of these options should be the output of the bundler?
1)
asyncapi: 3.0.0
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
userSignedUp:
address: user/signedup
messages:
UserSignedUp:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the user
operations:
onUserSignUp:
action: receive
channel:
$ref: '#/channels/userSignedUp'
messages:
- $ref: '#/channels/userSignedUp/messages/UserSignedUp'
components:
messages:
UserSignedUp:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the user
2)
asyncapi: 3.0.0
info:
title: Account Service
version: 1.0.0
description: This service is in charge of processing user signups
channels:
userSignedUp:
address: user/signedup
messages:
UserSignedUp:
$ref: '#/components/messages/UserSignedUp'
operations:
onUserSignUp:
action: receive
channel:
$ref: '#/channels/userSignedUp'
messages:
- $ref: '#/channels/userSignedUp/messages/UserSignedUp'
components:
messages:
UserSignedUp:
payload:
type: object
properties:
displayName:
type: string
description: Name of the user
email:
type: string
format: email
description: Email of the user
I think option 2 should be the result. as a user of bundler I don't expect bundler to de-reference my local references. If there is disagreement here, I think we should discuss this in a separate issue and see what the community wants.
should be rethought in a way that makes both options (in standalone and CLI) consistent?
Agreed.
I'll ask here since this issue already has the intended audience.
Should there be a separate mode in Bundler
that will dereference only external $ref
s, leaving internal $ref
s intact (previous behavior?)
Should there be a separate mode in Bundler that will dereference only external $refs, leaving internal $refs intact (previous behavior?)
cc: @Souvikns @derberg @peter-rr @mfroberg @francocm
I think it is reasonable use case to just dereference external refs and leave local ones intact. My original need is to get rid of the external refs (by bundling that content) so that I am able to visualize the file using Async Studio web tool.
Should there be a separate mode in
Bundler
that will dereference only external$ref
s, leaving internal$ref
s intact (previous behavior?)
IMO, that should be the default behaviour. In case we want to de-reference also the local refs, then we could add an option to bundle command for that purpose.
I will be able to implement bundle
after the onBundle
callback with the similar functionality as onDereference
is introduced in @apidevtools/json-schema-ref-parser
(I still need to do stuff after bundling and keep cmd switches consistent across versions.)
This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.
There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart:
Still relevant.
Waiting for https://github.com/asyncapi/cli/pull/1389 to be solved.
I'm looking forward to the fix as well. Currently it also kills the https://github.com/event-catalog/eventcatalog asyncapi plugin
@zbigniew-malcherczyk-tg
Hey. I am one of Bundler
's maintainers. Could you please point to the open issue?
Describe the bug.
The CLI tool now accepts asyncapi bundle for v3 documents (thanks to fix of issue https://github.com/asyncapi/cli/issues/1137). However, it does not work as I expected, but of course I could be missing something.
I created a new default document with
asyncapi new
and moved the single message to a file in the same directory. Added a $ref in the main file. When I ranasyncapi bundle main.yaml
on the new file, I expected it to merge the “message file”. But it didn’t.main.yaml
messages.yaml
Expected behavior
One yaml file containing the whole specification.
expected_bundle.yaml
Screenshots
How to Reproduce
main.yaml
andmessages.yaml
to the same directoryasyncapi bundle main.yaml
🥦 Browser
None
👀 Have you checked for similar open issues?
🏢 Have you read the Contributing Guidelines?
Are you willing to work on this issue ?
None