feat: add python readme #1725
Merged
feat: add python readme #1725
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thulieblack
requested review from
Adi-204,
asyncapi-bot-eve,
derberg and
magicmatatjahu
as code owners
🦋 Changeset detectedLatest commit: a07a0a4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds centralized README components (Readme, CoreMethods, Installation, Overview, Usage, MessageExamples, OperationHeader) to packages/components, updates component exports and tests, replaces manual README logic in the JavaScript websocket template with Readme, adds a Python Readme wrapper, and adjusts related package/config files and workflow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
What reviewer looks at during PR reviewThe following are ideal points maintainers look for during review. Reviewing these points yourself beforehand can help streamline the review process and reduce time to merge.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/templates/clients/websocket/python/template/README.md.js (3)
45-47: Document that connect() is async to match the example usage.The sample uses
await ws_client.connect()but the docs don't mention it's async.-#### `connect()` -Establishes a WebSocket connection to the server. +#### `connect()` (async) +Asynchronously establishes a WebSocket connection to the server.
87-93: Avoid referencing a likely non-existent method: send_echo_message.This name is too specific and may not exist in the generated client. Point users to actual operation methods.
- message = 'Hello, Echo!' + message = 'Hello, world!' while True: try: - await ws_client.send_echo_message(message) + await ws_client.<operation_method>(message) # Replace with a generated method from "Available Operations" above except Exception as error: print('Error while sending message:', error) await asyncio.sleep(interval)
77-77: Capitalize “WebSocket” correctly.- print('Errors from Websocket:', getattr(error, 'message', str(error))) + print('Errors from WebSocket:', getattr(error, 'message', str(error)))
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/templates/clients/websocket/python/template/README.md.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/templates/clients/websocket/python/template/README.md.js (3)
packages/helpers/src/servers.js (2)
getServer(34-45)getServerUrl(8-22)packages/helpers/src/utils.js (3)
getInfo(9-21)getClientName(53-61)getTitle(32-43)packages/templates/clients/websocket/javascript/components/AvailableOperations.js (1)
AvailableOperations(5-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test generator as dependency with Node 20
packages/templates/clients/websocket/python/template/README.md.js
Outdated
Show resolved
Hide resolved
packages/templates/clients/websocket/python/template/README.md.js
Outdated
Show resolved
Hide resolved
Member
Author
|
@Adi-204 I need to check how the readme renders but all the commands I have in my head are failing can you help me |
Member
|
@thulieblack you can check the templates rendered content by running integration test we have new integration test setup https://github.com/asyncapi/generator/tree/master/packages/templates/clients/websocket/test#integration-testing So basically cd does it answers your question? |
Member
Author
|
Not really, I don't want to run the integration tests, but I want to generate the Python README code locally and see how it displays |
Member
|
@thulieblack you want to see below README.md file right? 
Also when you added this new README.md their should be changes in snapshots which I don't see in this PR. I would suggest you to run |
Member
Author
|
Sorry Adi, could you please walk me through how you did this because I'm not reproducing this from my end. Also, I need to add the Readme to the snapshoot test as well? |
Member
|
@thulieblack can you please share final logs when you run |
Member
|
ok one thing I notice is in this PR ci logs test are failed still overall it is showing pass? 🤔 
cc @derberg can you also please look. |
Member
Author
|
In terms of temp content, this is what I see 
Logs after running After running Let me update my node version |
Member
|
yeah please update the node and after running 
|
Member
Author
|
thank you so much Adi, it worked 
|
Member
|
@thulieblack do you plan to add more commits to this PR? as for now I do not see it addressing the scope of the issue, the decomponentization |
Member
Author
|
Didn't you mention that I need to add create a PR for each component? https://asyncapi.slack.com/archives/C072JMTJ85Q/p1757513137652219?thread_ts=1757501093.164179&cid=C072JMTJ85Q Or do I add it on this PR alongside the new Python readme? |
Member
|
@thulieblack yes, but in your PR you just add a new readme template, no components, and do not reuse from the other readme - so you do not identify common parts and do not create components |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/templates/clients/websocket/python/components/Overview.js (1)
10-12: Skip bullets when metadata is missingWhen the AsyncAPI document has no default server (or the helper fails to resolve one) the README currently prints
- **Server URL:** undefined, which looks broken and undermines the componentization effort. Same risk applies toversionif the input omits it. Better to add those bullets only when the values exist.Proposed tweak:
- {`# ${title} - -## Overview - -${description || `A WebSocket client for ${title}.`} -- **Version:** ${version} -- **Server URL:** ${serverUrl} -`} + {[ + `# ${title}`, + '', + '## Overview', + '', + description || `A WebSocket client for ${title}.`, + version ? `- **Version:** ${version}` : null, + serverUrl ? `- **Server URL:** ${serverUrl}` : null, + ] + .filter(Boolean) + .join('\n')}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/templates/clients/websocket/test/integration-test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
packages/templates/clients/websocket/python/components/Overview.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test NodeJS PR - macos-13
- GitHub Check: Test NodeJS PR - ubuntu-latest
- GitHub Check: Test NodeJS PR - windows-latest
Member
Author
|
@derberg I did follow the example can you check again unless im missing something |
Adi-204
previously approved these changes
Dec 4, 2025
Member
There was a problem hiding this comment.
@thulieblack please change PR title and add feat: instead of chore: as mention in #1725 (comment) point 1.
derberg
reviewed
Dec 4, 2025
Co-authored-by: Lukasz Gornicki <[email protected]>
derberg
reviewed
Dec 4, 2025
Member
derberg
left a comment
derberg
left a comment
There was a problem hiding this comment.
there is one thing to clarify by @Adi-204 and we can merge
please add one more change, from https://github.com/asyncapi/generator/blob/master/.github/workflows/release-with-changesets.yml#L98 to uses: asyncapi/.github/.github/actions/get-node-version-from-package-lock@master
Member
Author
Should I wait for Adi on this or I need to make the change @derberg |
derberg
reviewed
Dec 8, 2025
Member
derberg
left a comment
derberg
left a comment
There was a problem hiding this comment.
@thulieblack all good just 2 things to do
- create folloup issues, tasks to work on
AvailableOperationsandInstallationso they are not hardcoded to just one language. 2 separate issues - for GenerateReadme.js and GenerateReadMe.js please perform rename. Just call file
Readme.jsand componentReadmeto stay consistent with other components naming
Member
Author
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/components/src/components/readme/Readme.js (1)
1-34: Readme composition and conditional sections look solidThe Readme component cleanly wires together Overview, Installation, Usage, CoreMethods, and AvailableOperations, and the language-based flags on Line 16–Line 18 keep Python and JavaScript behavior straightforward. Computing
operationsonly whenlanguage === 'javascript'(Line 18) avoids unnecessary work for other languages.Only small ask: double‑check that all templates invoking this component always provide the expected
paramsfields (server,clientFileName,appendClientSuffix,customClientName) so README generation doesn’t accidentally renderundefinedvalues if a template diverges.If you want, I can help sketch a tiny guard or defaulting layer around
paramsto make this component more defensive.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/components/src/components/readme/Readme.js(1 hunks)packages/components/src/index.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-23T09:18:38.333Z
Learnt from: derberg
Repo: asyncapi/generator PR: 1512
File: packages/templates/clients/websocket/javascript/components/AvailableOperations.js:10-15
Timestamp: 2025-04-23T09:18:38.333Z
Learning: In the asyncapi/generator repository, keys aren't required when mapping over operations array in the AvailableOperations component, contrary to the typical React pattern.
Applied to files:
packages/components/src/components/readme/Readme.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-latest
- GitHub Check: Test NodeJS PR - ubuntu-latest
🔇 Additional comments (1)
packages/components/src/index.js (1)
15-20: New readme-related exports are consistent and minimalThe additional exports for
RegisterErrorHandlerand the readme components (Line 15–Line 20) are coherent with the existing barrel pattern and keep naming aligned with the underlying files. No issues from an API-surface or maintainability perspective.
Member
|
you forgot about #1725 (comment) |
|
derberg
approved these changes
Dec 11, 2025
Member
|
/rtm |
This was referenced Dec 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of the componetization of the readme, this PR adds a readme to the python template.
relates #1524
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.