Patch python strings in java - #14044
Conversation
…lates in createKubernetesCluster Add usage examples to the @parameter descriptions for nodeofferings and nodetemplates in CreateKubernetesClusterCmd.java, showing how to specify different service offerings and templates for control vs worker nodes. Discussion: apache#13395
replace `\”` with `’`
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14044 +/- ##
============================================
- Coverage 19.78% 19.78% -0.01%
+ Complexity 19998 19988 -10
============================================
Files 6371 6371
Lines 575899 575899
Branches 70495 70495
============================================
- Hits 113967 113915 -52
- Misses 449501 449557 +56
+ Partials 12431 12427 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The updated examples include single quotes around values, which would be interpreted literally if copied into real API requests and can mislead users.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the CloudStack Kubernetes cluster creation API parameter descriptions to avoid problematic quoting/escaping in generated documentation (as described, Java strings being interpreted/escaped incorrectly in a Python-based generation step).
Changes:
- Adjusted the
nodeofferings[...]example string inNODE_TYPE_OFFERING_MAPparameter documentation. - Adjusted the
nodetemplates[...]example string inNODE_TYPE_TEMPLATE_MAPparameter documentation.
File summaries
| File | Description |
|---|---|
| plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/CreateKubernetesClusterCmd.java | Updates API parameter description examples for node offering/template maps to change quoting/escaping behavior in generated docs. |
Review details
Suppressed comments (1)
plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/user/kubernetes/cluster/CreateKubernetesClusterCmd.java:129
- The nodetemplates example uses single quotes around values (node='control', template=''), which would be interpreted as literal quotes if copied into a real request. To avoid misleading API users (and to match other examples in this class), remove the quotes and show raw values/placeholders instead.
description = "(Optional) Node Type to Template ID mapping. If provided, it overrides the default template: System VM template. " +
"Example: nodetemplates[0].node='control'&nodetemplates[0].template='<template-uuid>'&" +
"nodetemplates[1].node='worker'&nodetemplates[1].template='<template-uuid>'",
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| description = "(Optional) Node Type to Service Offering ID mapping. If provided, it overrides the serviceofferingid parameter. " + | ||
| "Example: nodeofferings[0].node=\"control\"&nodeofferings[0].offering=\"<service-offering-uuid>\"&" + | ||
| "nodeofferings[1].node=\"worker\"&nodeofferings[1].offering=\"<service-offering-uuid>\"", | ||
| "Example: nodeofferings[0].node='control'&nodeofferings[0].offering='<service-offering-uuid>'&" + | ||
| "nodeofferings[1].node='worker'&nodeofferings[1].offering='<service-offering-uuid>'", |
Description
This PR solved a code generated issue where java strings are interpreted in python and this break escaping. an alternative is use puthon multiline strings which would work as java concatenated string, but would be confusing (i.e. ‘\”.”””’)
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?