refactor: replace hardcode sendEchoMessage method in py websocket client
#1572
Conversation
Adi-204
requested review from
derberg,
magicmatatjahu,
jonaslagoni and
asyncapi-bot-eve
as code owners
|
|
""" WalkthroughThe changes refactor the Python WebSocket client code generation to dynamically create send methods for each operation defined in the AsyncAPI document. The previous static Changes
Sequence Diagram(s)sequenceDiagram
participant AsyncAPIDoc as AsyncAPI Document
participant TemplateGen as Template Generator
participant ClientClass as ClientClass Component
participant SendOperation as SendOperation Component
participant Send as Send Component
AsyncAPIDoc->>TemplateGen: Provide operations
TemplateGen->>ClientClass: Pass operations prop
ClientClass->>SendOperation: Render send methods for each operation
ClientClass->>Send: Render shared _send helper if operations exist
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/templates/clients/websocket/python/components/ClientClass.js (1)
26-27: Add null check for sendOperations to avoid potential runtime errorsThe conditional check at line 27 assumes
sendOperationsis always defined, but there could be cases where it's undefined or null when passed to this component.- {sendOperations.length > 0 && <Send />} + {sendOperations && sendOperations.length > 0 && <Send />}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
packages/templates/clients/websocket/python/test/__snapshots__/integration.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
packages/templates/clients/websocket/python/components/ClientClass.js(2 hunks)packages/templates/clients/websocket/python/components/Send.js(1 hunks)packages/templates/clients/websocket/python/components/SendEchoMessage.js(0 hunks)packages/templates/clients/websocket/python/components/SendOperation.js(1 hunks)packages/templates/clients/websocket/python/example.py(1 hunks)packages/templates/clients/websocket/python/template/client.py.js(2 hunks)packages/templates/clients/websocket/test/python/test_acceptance.py(1 hunks)
💤 Files with no reviewable changes (1)
- packages/templates/clients/websocket/python/components/SendEchoMessage.js
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/templates/clients/websocket/python/components/ClientClass.js (2)
packages/templates/clients/websocket/python/components/SendOperation.js (1)
SendOperation(3-48)packages/templates/clients/websocket/python/components/Send.js (1)
Send(3-29)
packages/templates/clients/websocket/python/components/SendOperation.js (1)
packages/templates/clients/websocket/python/template/client.py.js (2)
sendOperations(14-14)clientName(12-12)
🪛 Biome (1.9.4)
packages/templates/clients/websocket/python/components/SendOperation.js
[error] 15-15: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (9)
packages/templates/clients/websocket/python/components/Send.js (1)
1-29: Well-structured helper method with good error handlingThe new
Sendcomponent implements a clean, reusable helper method for WebSocket message sending with proper error handling and documentation. The method correctly handles different message types, converting dictionaries to JSON automatically. This implementation provides a good foundation for the operation-specific send methods.packages/templates/clients/websocket/python/example.py (1)
34-34: Method name updated to match new conventionThe update from the generic
send_messageto the operation-specificsendEchoMessagecorrectly aligns with the refactoring approach.packages/templates/clients/websocket/test/python/test_acceptance.py (1)
66-66: Test updated to use new operation-specific method nameThe test now correctly calls
sendEchoMessage_staticinstead of the previous generic method, maintaining compatibility with the refactored code while preserving the test's functionality.packages/templates/clients/websocket/python/template/client.py.js (2)
14-14: Key improvement: Extract send operations from the AsyncAPI documentThis change enables the dynamic generation of operation-specific send methods by extracting actual send operations from the document rather than using hardcoded methods.
23-23: Pass extracted operations to ClientClass componentProperly passing the send operations to the ClientClass component completes the refactoring, allowing the component to generate appropriate send methods based on the AsyncAPI document's specifications.
packages/templates/clients/websocket/python/components/ClientClass.js (2)
7-8: Good architecture change: Replacing hardcoded send method with dynamic componentsThe imports reflect a more modular approach by separating concerns between operation-specific send methods and the underlying send functionality. This is a good design choice that improves flexibility and maintainability.
13-13: LGTM: Updated function signature to support dynamic operationsAdding the
sendOperationsparameter enables dynamic generation of operation-specific methods based on the AsyncAPI definition.packages/templates/clients/websocket/python/components/SendOperation.js (2)
3-6: LGTM: Good defensive programming with guard clauseThe function correctly handles the case when no send operations are available, returning null early and avoiding unnecessary processing.
16-41: LGTM: Well-structured method generation with comprehensive documentationThe generated Python code for both instance and static methods includes:
- Descriptive docstrings explaining purpose and usage
- Proper argument descriptions
- Clear exception documentation
- Consistent naming conventions
This makes the generated client API both user-friendly and maintainable.
packages/templates/clients/websocket/python/components/SendOperation.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/templates/clients/websocket/python/test/components/SendOperation.test.js (1)
7-7: Consider using a more maintainable fixture path.The deep relative path to the fixture file (
../../../../../../helpers/test/__fixtures__/asyncapi-websocket-query.yml) could make the test brittle if the project structure changes.Consider using a helper function or configuration to locate test fixtures more robustly:
-const asyncapi_websocket_query = path.resolve(__dirname, '../../../../../../helpers/test/__fixtures__/asyncapi-websocket-query.yml'); +// Import a helper that manages fixture paths +import { getFixturePath } from '../../../test-utils'; +const asyncapi_websocket_query = getFixturePath('asyncapi-websocket-query.yml');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/templates/clients/websocket/python/test/components/__snapshots__/SendOperation.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
packages/templates/clients/websocket/python/test/components/SendOperation.test.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- 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
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (4)
packages/templates/clients/websocket/python/test/components/SendOperation.test.js (4)
1-8: Setup looks good and follows best practices.The imports and test setup are well-structured, utilizing the AsyncAPI parser and fixture files appropriately.
9-16: Well-structured test suite initialization.The test suite is properly organized with a
beforeAllhook that parses the AsyncAPI document once before all tests run, which is efficient.
17-26: Good test case for the primary use case.This test effectively verifies the component's behavior when provided with valid send operations, using snapshot testing to validate the output.
28-38: Edge case handling is properly tested.The test for rendering without send operations ensures the component gracefully handles the absence of operations, which is an important edge case to cover.
| @@ -19,7 +20,7 @@ export default function ({ asyncapi, params }) { | |||
| server={server} | |||
| /> | |||
| <Requires query={queryParams} /> | |||
| <ClientClass clientName={clientName} serverUrl={serverUrl} title={title} queryParams={queryParams} /> | |||
| <ClientClass clientName={clientName} serverUrl={serverUrl} title={title} queryParams={queryParams} sendOperations={sendOperations} /> | |||
There was a problem hiding this comment.
here we should pass only operations, as later anyway we will need to also generate receive action methods, and related component will need access to operations as well
| @@ -22,7 +23,8 @@ export function ClientClass({ clientName, serverUrl, title, queryParams }) { | |||
| <RegisterOutgoingProcessor /> | |||
| <HandleMessage /> | |||
| <HandleError /> | |||
| <SendEchoMessage /> | |||
| <SendOperation sendOperations={sendOperations} clientName={clientName} /> | |||
There was a problem hiding this comment.
should we call the component SendOperation influenced by the name of object it operates on, or rather SendMethods or SendFunctions or event OutgoingFunctions
There was a problem hiding this comment.
So according to me if someone is developing template or want to modify our template code, I think they might refer to their own AsyncAPI document frequently in which word "operations" will be mentioned so to keep name like SendOperations I think it is more intuitive and one can easily understand.
But you have a lot of experience and talk with companies which are using AsyncAPI. wdyt about my point does it make sense? 🤔 interested to know your thoughts.
| @@ -22,7 +23,8 @@ export function ClientClass({ clientName, serverUrl, title, queryParams }) { | |||
| <RegisterOutgoingProcessor /> | |||
| <HandleMessage /> | |||
| <HandleError /> | |||
| <SendEchoMessage /> | |||
| <SendOperation sendOperations={sendOperations} clientName={clientName} /> | |||
| {sendOperations.length > 0 && <Send />} | |||
There was a problem hiding this comment.
I know Send do not require any arguments, but wouldn't it be easier and cleaner to use if it actually accepts sendOperations as input and do sendOperations.length > 0 internally ?
…place-hardcodMethod
So I read about operations id from - https://www.asyncapi.com/docs/reference/specification/v3.0.0#operationsObject |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/templates/clients/websocket/python/components/SendOperation.js (2)
8-13: Good implementation of snake_case conversion.This function correctly addresses the Python naming convention requirements mentioned in the PR objectives. The logic handles the main use cases well (camelCase, PascalCase, single words).
For edge cases with consecutive capitals (e.g., "XMLHttpRequest"), consider this alternative if needed:
const toSnakeCase = (camelStr) => { return camelStr - .replace(/([A-Z])/g, '_$1') + .replace(/([A-Z]+)([A-Z][a-z])/g, '$1_$2') + .replace(/([a-z\d])([A-Z])/g, '$1_$2') .toLowerCase() .replace(/^_/, ''); };However, the current implementation is likely sufficient for typical operation IDs.
23-56: Well-structured Python code generation with minor docstring improvement needed.The generated Python methods are well-implemented with proper async syntax, docstrings, and error handling documentation.
However, there's a minor inconsistency in the static method docstring:
- Send a ${staticMethodName} message using a provided WebSocket connection, without needing an instance. + Send a ${methodName} message using a provided WebSocket connection, without needing an instance.This avoids including "_static" in the user-facing message description, making it more natural to read.
The overall approach successfully replaces the hardcoded approach with dynamic method generation as intended.
🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/templates/clients/websocket/python/test/__snapshots__/integration.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/python/test/components/__snapshots__/SendOperation.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
packages/templates/clients/websocket/python/components/ClientClass.js(2 hunks)packages/templates/clients/websocket/python/components/Send.js(1 hunks)packages/templates/clients/websocket/python/components/SendOperation.js(1 hunks)packages/templates/clients/websocket/python/example.py(1 hunks)packages/templates/clients/websocket/python/template/client.py.js(2 hunks)packages/templates/clients/websocket/test/python/test_acceptance.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/templates/clients/websocket/python/example.py
- packages/templates/clients/websocket/test/python/test_acceptance.py
- packages/templates/clients/websocket/python/components/ClientClass.js
- packages/templates/clients/websocket/python/components/Send.js
- packages/templates/clients/websocket/python/template/client.py.js
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/templates/clients/websocket/python/components/SendOperation.js (2)
packages/templates/clients/websocket/python/components/ClientClass.js (1)
sendOperations(14-14)packages/templates/clients/websocket/python/template/client.py.js (1)
clientName(12-12)
🪛 Biome (1.9.4)
packages/templates/clients/websocket/python/components/SendOperation.js
[error] 23-23: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Test generator as dependency with Node 20
🔇 Additional comments (2)
packages/templates/clients/websocket/python/components/SendOperation.js (2)
1-6: LGTM! Clean component structure.The import and early return logic are well-implemented, following React best practices.
15-22: Mapping logic is correctly implemented.The structure properly generates both instance and static method names using the snake_case conversion. The approach aligns well with the PR's goal of dynamic method generation.
Note: I'm aware of the static analysis hint about missing keys, but based on previous feedback, this is not applicable in this context.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/templates/clients/websocket/python/components/SendOperation.js (1)
20-20: Fix extra spacing in template string.There's an extra space before the closing brace that will affect the generated method name.
Apply this diff to fix the spacing:
- const staticMethodName = `${toSnakeCase(operation.id()) }_static`; + const staticMethodName = `${toSnakeCase(operation.id())}_static`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/templates/clients/websocket/python/test/__snapshots__/integration.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/python/test/components/__snapshots__/SendOperation.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
packages/templates/clients/websocket/python/components/SendOperation.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/templates/clients/websocket/python/components/SendOperation.js (2)
packages/templates/clients/websocket/python/components/ClientClass.js (1)
sendOperations(14-14)packages/templates/clients/websocket/python/template/client.py.js (1)
clientName(12-12)
🪛 Biome (1.9.4)
packages/templates/clients/websocket/python/components/SendOperation.js
[error] 23-23: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- 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
🔇 Additional comments (4)
packages/templates/clients/websocket/python/components/SendOperation.js (4)
1-2: LGTM!The import statement is correct and follows the expected pattern for AsyncAPI generator components.
3-6: LGTM!Good defensive programming with the early return for empty or undefined
sendOperations. The function signature correctly accepts the required props.
8-13: Excellent implementation of snake_case conversion!This function perfectly addresses the PR objective of converting camelCase/PascalCase operation IDs to Python's snake_case convention. The logic correctly:
- Adds underscores before capital letters
- Converts to lowercase
- Removes any leading underscore for PascalCase inputs
- Leaves single lowercase words unchanged
This handles all the cases mentioned in the PR comments.
22-55: Well-structured Python method generation!The generated Python methods follow excellent practices:
- Proper async/await syntax for WebSocket operations
- Comprehensive docstrings with Args and Raises sections
- Clear separation between instance and static methods
- Correct delegation to the shared
_sendhelper methodThe template correctly generates both patterns needed for the WebSocket client.
🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/templates/clients/websocket/python/components/SendOperation.js (1)
8-13: Improve the toSnakeCase function to handle edge cases.The current implementation works well for typical camelCase/PascalCase inputs but has issues with consecutive capital letters.
For example:
"XMLHttpRequest"becomes"x_m_l_http_request""SENDMESSAGE"becomes"s_e_n_d_m_e_s_s_a_g_e"Consider this improved implementation:
-const toSnakeCase = (camelStr) => { - return camelStr - .replace(/([A-Z])/g, '_$1') - .toLowerCase() - .replace(/^_/, ''); -}; +const toSnakeCase = (camelStr) => { + return camelStr + .replace(/([A-Z]+)([A-Z][a-z])/g, '$1_$2') + .replace(/([a-z\d])([A-Z])/g, '$1_$2') + .toLowerCase(); +};This handles consecutive capitals better:
"XMLHttpRequest"→"xml_http_request""sendMessage"→"send_message""SendMessage"→"send_message"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/templates/clients/websocket/python/components/SendOperation.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/templates/clients/websocket/python/components/SendOperation.js (2)
packages/templates/clients/websocket/python/components/ClientClass.js (1)
sendOperations(14-14)packages/templates/clients/websocket/python/template/client.py.js (1)
clientName(12-12)
🪛 Biome (1.9.4)
packages/templates/clients/websocket/python/components/SendOperation.js
[error] 23-23: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- 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-13
🔇 Additional comments (2)
packages/templates/clients/websocket/python/components/SendOperation.js (2)
4-6: LGTM! Proper early return handling.Good defensive programming practice to handle null or empty sendOperations early.
18-52: LGTM! Well-structured method generation with comprehensive documentation.The mapping logic effectively generates both instance and static methods with:
- Proper Python async syntax
- Comprehensive docstrings following Python conventions
- Consistent parameter handling
- Clear separation between instance and static method variants
The generated methods follow Python naming conventions and provide good documentation for API consumers.
🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
| return null; | ||
| } | ||
|
|
||
| const toSnakeCase = (camelStr) => { |
There was a problem hiding this comment.
we have some code for inspiration already in modelina
https://github.com/asyncapi/modelina/blob/master/src/helpers/FormatHelpers.ts#L243
anyway, please make sure it is a separate helper, we need to have it unit tested, to make sure it doesn't do anything wrong with existing snake case, if somebody passes it into operationId by default
There was a problem hiding this comment.
oh ok I will add a new helper @derberg but for helpers tester we are using fixture only but for this one we would have to test it directly with different possible strings or I adjust asyncapi document and perform testing with operationid
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/templates/clients/websocket/python/components/SendOperation.js (1)
1-50: Well-structured component for dynamic send method generation.This component successfully replaces the hardcoded
sendEchoMessageapproach with a flexible, operation-driven solution. The implementation:
- Correctly uses
toSnakeCaseto convert operation IDs to Python-friendly method names- Generates both instance and static methods for each operation, providing flexibility
- Includes comprehensive docstrings with proper Args and Raises sections
- Handles edge cases (null/empty operations) gracefully
- Uses clean template literals for readable Python code generation
The generated Python methods follow good practices by calling the shared
_sendhelper method, promoting code reuse.Note: The static analysis warning about missing React keys can be ignored as it was previously determined to be incorrect for this codebase context.
🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🧹 Nitpick comments (1)
packages/helpers/test/__fixtures__/asyncapi-websocket-query.yml (1)
140-153: Fix trailing whitespace in the YAML fixture.The new test operations are well-designed to test different casing styles (PascalCase and snake_case), which aligns perfectly with the
toSnakeCasefunction requirements.However, there's a formatting issue that needs to be addressed:
- +🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 146-146: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/templates/clients/websocket/javascript/test/components/__snapshots__/AvailableOperations.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/javascript/test/components/__snapshots__/SendOperation.test.js.snapis excluded by!**/*.snappackages/templates/clients/websocket/python/test/components/__snapshots__/SendOperation.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
packages/helpers/src/index.js(2 hunks)packages/helpers/src/utils.js(1 hunks)packages/helpers/test/__fixtures__/asyncapi-websocket-query.yml(1 hunks)packages/helpers/test/utils.test.js(2 hunks)packages/templates/clients/websocket/python/components/SendOperation.js(1 hunks)packages/templates/clients/websocket/python/template/client.py.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/helpers/src/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/templates/clients/websocket/python/template/client.py.js
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/helpers/test/utils.test.js (2)
packages/helpers/src/index.js (4)
require(1-1)require(2-2)require(3-3)require(4-4)packages/helpers/src/utils.js (2)
require(1-1)toSnakeCase(67-76)
packages/templates/clients/websocket/python/components/SendOperation.js (3)
packages/templates/clients/websocket/python/components/ClientClass.js (1)
sendOperations(14-14)packages/helpers/src/utils.js (1)
toSnakeCase(67-76)packages/templates/clients/websocket/python/template/client.py.js (1)
clientName(12-12)
🪛 YAMLlint (1.37.1)
packages/helpers/test/__fixtures__/asyncapi-websocket-query.yml
[error] 146-146: trailing spaces
(trailing-spaces)
🪛 Biome (1.9.4)
packages/templates/clients/websocket/python/components/SendOperation.js
[error] 17-17: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test generator as dependency with Node 18
🔇 Additional comments (2)
packages/helpers/src/utils.js (1)
60-77: Well-implemented utility function for case conversion.The
toSnakeCasefunction correctly addresses the PR objective of converting camelCase/PascalCase operation IDs to Python-friendly snake_case method names. The implementation properly:
- Preserves already snake_case strings using regex validation
- Handles camelCase/PascalCase conversion by splitting before uppercase letters
- Manages non-word characters by replacing them with spaces first
- Provides clear documentation
packages/helpers/test/utils.test.js (1)
85-120: Comprehensive test coverage for the toSnakeCase function.The test suite excellently covers all key scenarios:
- PascalCase to snake_case conversion (
PascalCaseOperation→pascal_case_operation)- Preserving existing snake_case format (
operation_with_snake_caseunchanged)- camelCase conversion (
noSummaryNoDescriptionOperations→no_summary_no_description_operations)- Edge case handling (empty string)
Using real AsyncAPI operations from the fixture makes the tests realistic and ensures integration works correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/helpers/test/__fixtures__/asyncapi-websocket-query.yml (2)
146-146: Remove trailing whitespaceThere are trailing spaces on the blank line after the
PascalCaseOperationentry. Removing these will satisfy YAML lint rules and avoid unnecessary diffs.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 146-146: trailing spaces
(trailing-spaces)
163-163: Add newline at end of fileYAML files should end with a single newline character. Please ensure the fixture file has a newline at the end to satisfy the YAML spec and linting rules.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 163-163: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/helpers/test/__fixtures__/asyncapi-websocket-query.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/helpers/test/__fixtures__/asyncapi-websocket-query.yml
[error] 146-146: trailing spaces
(trailing-spaces)
[error] 163-163: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test generator as dependency with Node 18
🔇 Additional comments (3)
packages/helpers/test/__fixtures__/asyncapi-websocket-query.yml (3)
140-146: Approve addition ofPascalCaseOperationfixture entryAdding the
PascalCaseOperationreceive operation in the test fixture ensures coverage for PascalCase IDs and will validate the snake_case conversion in the Python client generator. The structure and indentation align with existing entries.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 146-146: trailing spaces
(trailing-spaces)
147-153: Approve addition ofoperation_with_snake_casefixture entryIncluding a send operation already in snake_case (
operation_with_snake_case) is a great way to verify that existing snake_case IDs remain unchanged through the conversion logic. The entry is well-formed and consistent with other send operations in the fixture.
163-163: Approve addition ofethusdenum valueExtending the symbol parameter enum with
ethusdin the components section improves test coverage for the path variable’s allowed values. The indentation matches the other enum items.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 163-163: no new line character at the end of file
(new-line-at-end-of-file)
|
@derberg I had to add 2 new operations in file for testing my toSnakeCase and hence integration test of some other part is also updated. I could change existing ones but I thought because multiple contributors are working on project I will break someone else testing code. |
|
|
/rtm |
Description
Replace hardcode
sendEchoMessagemethod inpywebsocket client.Related issue(s)
Fixes #1517
Summary by CodeRabbit
New Features
Bug Fixes
Refactor