Skip to content

Fix bug in AsyncAPI schema #2638

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Nov 11, 2025
Merged

Fix bug in AsyncAPI schema #2638

merged 10 commits into from
Nov 11, 2025

Conversation

@kittywaresz
Copy link
Contributor

@kittywaresz kittywaresz commented Nov 2, 2025 •
edited
Loading

Fix bug in AsyncAPI schema

No extra "Subscribe" suffix in AsyncAPI operations object if title parameter was specified explicitly during subscriber creation

Fixes #2617

Type of change

Please delete options that are not relevant.

  • Bug fix (a non-breaking change that resolves an issue)

Checklist

  • My code adheres to the style guidelines of this project (just lint shows no errors)
  • I have conducted a self-review of my own code
  • I have made the necessary changes to the documentation
  • My changes do not generate any new warnings
  • I have added tests to validate the effectiveness of my fix or the functionality of my new feature
  • Both new and existing unit tests pass successfully on my local environment by running just test-coverage
  • I have ensured that static analysis tests are passing by running just static-analysis
  • I have included code examples to illustrate the modifications

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Nov 2, 2025
@kittywaresz kittywaresz changed the title Issue-2617: Fix bug in AsyncAPI schema Fix bug in AsyncAPI schema Nov 2, 2025
@kittywaresz kittywaresz mentioned this pull request Nov 2, 2025
Closed
@kittywaresz
Copy link
Contributor Author

kittywaresz commented Nov 2, 2025

Please take a look at #2617 (comment) before review

Lancetnik
Lancetnik requested changes Nov 2, 2025
View reviewed changes
Copy link
Member

@Lancetnik Lancetnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but this is an incorrect solution. We should add the "Subscribe" suffix to the handlers' name in the default case. However, if the user creates a subscriber with a manual name assignment, such as broker.subscriber(..., title="some-title"), we should set it up as is, without any mangling.

Also, please revert any changes doesn't relate to the Issue

kittywaresz
kittywaresz commented Nov 2, 2025
View reviewed changes
tests/asyncapi/base/v3_0_0/arguments.py
assert schema["channels"][channel_key]["address"] == "/"

assert next(iter(schema["operations"].keys())) == ".Subscribe"
assert operation_key == ".Subscribe"
Copy link
Contributor Author

@kittywaresz kittywaresz Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that title="/" is a special case and we need mangling here

@kittywaresz kittywaresz requested a review from Lancetnik November 2, 2025 20:39
Lancetnik
Lancetnik reviewed Nov 6, 2025
View reviewed changes
@kittywaresz kittywaresz requested a review from Lancetnik November 7, 2025 14:53
@Lancetnik
Remove schema assertions for channel and operation keys
b61d39e 
Removed assertions checking for channel and operation keys in the schema.
Lancetnik
Lancetnik previously approved these changes Nov 7, 2025
View reviewed changes
@Lancetnik
Copy link
Member

Lancetnik commented Nov 7, 2025

@kittywaresz can you please fix CI?

@kittywaresz
Copy link
Contributor Author

kittywaresz commented Nov 8, 2025

@Lancetnik fixed

Can you please check my old comment #2439 (comment)? It seems that pre-commit action not configured properly, I think it should be able to commit changes to fix simple issues like this one

@Lancetnik
Copy link
Member

Lancetnik commented Nov 11, 2025

@Lancetnik fixed

Can you please check my old comment #2439 (comment)? It seems that pre-commit action not configured properly, I think it should be able to commit changes to fix simple issues like this one

Yes, but I don't know, how to configure it correctly:
Previously we had two edge cases

  1. It commits, but doesn't fail in cases can't fix
  2. It fails, but can't commit

@Lancetnik Lancetnik added this pull request to the merge queue Nov 11, 2025
Merged via the queue into ag2ai:main with commit cce5bf3 Nov 11, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lancetnik Lancetnik Lancetnik approved these changes

@kumaranvpl kumaranvpl Awaiting requested review from kumaranvpl

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: additional "Subscribe" in docs

2 participants

@kittywaresz @Lancetnik