-
Notifications
You must be signed in to change notification settings - Fork 301
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.
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
HPCC-29963 Correct XSD type conversion for TpQueryType in WsTopology Service #17612
Conversation
https://track.hpccsystems.com/browse/HPCC-29963 |
esp/xslt/esp_service_xml2xsd.xslt
Outdated
@@ -153,6 +153,8 @@ | |||
</xsl:when> | |||
<xsl:when test="@type='int64'">xsd:<xsl:value-of select="'long'"/> | |||
</xsl:when> | |||
<xsl:when test="@type='TpQueryType'">xsd:<xsl:value-of select="'string'"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kenrowland This is actually covering up a bug in ws_topology.ecm. TpQueryType was meant to be defined as an enumerated string. But it was incorrectly defined and incorrectly used.
You could change the definition of TpQueryType from ESPstruct to ESPenum and then make use of it, BUT that may cause backward compatibility issues.
The alternative for maximum backward compatibility would be just to change it to a string.
@wangkx when you were changing strings to enums how did we handle backward compatibility?
Anyway I wouldn't make this change, one way or the other I would fix the bad item in the ecm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In ecm file, add a new variable and 'depr_ver' the old one. If the variables are in a request, make sure that the new variable is read first. If not found, read the old one for backward compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kenrowland for now I would just declare it as a string but add a comment about the two expected values (from the struct definition). It's no worse or less informative than what is there now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't hard code this change, rather fix the errors in the ecm.
@kenrowland the PR and commit titles and comments are no longer correct. |
@kenrowland can you squash and double check the commit message? |
…rvice Changed TpQueryType to string to produce correct type in XSD Signed-Off-By: Kenneth Rowland [email protected]
@GordonSmith Please merge |
Added exception for type TpQueryType in XSLT to ensure generated XSD has proper type of xsd:string
Signed-Off-By: Kenneth Rowland [email protected]
Type of change:
Checklist:
Smoketest:
Testing: