Skip to content
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

HPCC-30429 Fix WsECL display for 'Create Workunit' option #17887

Merged
merged 1 commit into from
Nov 6, 2023
Merged

HPCC-30429 Fix WsECL display for 'Create Workunit' option #17887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions esp/services/ws_ecl/ws_ecl_service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1568,12 +1568,17 @@ int CWsEclBinding::getGenForm(IEspContext &context, CHttpRequest* request, CHttp
xform->setParameter("useTextareaForStringArray", "1");

#ifdef _CONTAINERIZED
bool isRoxie = wsecl->connMap.getValue(wuinfo.qsetname) != nullptr;
#else
VStringBuffer xpath("queues[@name='%s']", wuinfo.qsetname.get());
IPropertyTree *queue = getComponentConfigSP()->queryPropTree(xpath.str());
if (queue && strieq(queue->queryProp("@type"), "roxie"))
xform->setParameter("includeRoxieOptions", queue->getPropBool("@queriesOnly") ? "2" : "3");
Copy link
Member

Choose a reason for hiding this comment

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

@afishbeck is this correct. Does queriesOnly imply that you cannot deploy a query? Should it return "1" or "3"?

Copy link
Member

Choose a reason for hiding this comment

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

Deploying a query isn't the right term here.

In k8s we separated out the roxies that would listen on a workunit queue to run workunits.

So for queriesOnly, you can deploy a query and then send it xml/json/testsocket requests. But you can't submit and run workunits.

The opposites is also currently true. Roxie-workunit components are setup like eclagents and you can't deploy queries to them.

Roxie itself supports mixed mode so we could easily change that or make it an option (if the option isn't already there) on the roxie side. We may need a new flag to specify mixed mode in the config so that this code and other places can know the difference though.

else
xform->setParameter("includeRoxieOptions", "0");
#else
Owned<IConstWUClusterInfo> clusterInfo = getTargetClusterInfo(wuinfo.qsetname);
bool isRoxie = clusterInfo && (clusterInfo->getPlatform() == RoxieCluster);
#endif
xform->setParameter("includeRoxieOptions", isRoxie ? "1" : "0");
#endif

// set the prop noDefaultValue param
IProperties* props = context.queryRequestParameters();
Expand Down Expand Up @@ -2134,8 +2139,18 @@ void CWsEclBinding::sendRoxieRequest(const char *target, StringBuffer &req, Stri
}
}

static void checkWorkunitCompatibleOptions(IEspContext &context, bool submitWorkunit)
{
bool statsToWorkunit = context.queryRequestParameters()->getPropBool("@statsToWorkunit", false);
bool summaryStats = context.queryRequestParameters()->getPropBool("@summaryStats", false);
if (submitWorkunit && (statsToWorkunit || summaryStats))
throw makeStringException(-1, "Neither 'Save stats to workunit' or 'Get summary stats' are supported for the 'Create Workunit' option");
}

int CWsEclBinding::onSubmitQueryOutput(IEspContext &context, CHttpRequest* request, CHttpResponse* response, WsEclWuInfo &wsinfo, const char *format, bool forceCreateWorkunit)
{
checkWorkunitCompatibleOptions(context, forceCreateWorkunit);

StringBuffer status;
StringBuffer output;

Expand Down Expand Up @@ -2200,6 +2215,8 @@ int CWsEclBinding::onSubmitQueryOutput(IEspContext &context, CHttpRequest* reque

int CWsEclBinding::onSubmitQueryOutputView(IEspContext &context, CHttpRequest* request, CHttpResponse* response, WsEclWuInfo &wsinfo, bool forceCreateWorkunit)
{
checkWorkunitCompatibleOptions(context, forceCreateWorkunit);

IConstWorkUnit *wu = wsinfo.ensureWorkUnit();

StringBuffer soapmsg;
Expand Down
12 changes: 11 additions & 1 deletion esp/xslt/wsecl3_form.xsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ function switchInputForm()
</span>
</td>
</tr>
<xsl:if test="$includeRoxieOptions=1">
<xsl:if test="$includeRoxieOptions=1 or $includeRoxieOptions=2">
Copy link
Member

Choose a reason for hiding this comment

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

Should this be != 0? Currently 3 is not included.

Copy link
Member Author

Choose a reason for hiding this comment

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

The 3 is not included here because it is for a roxie which always runs a published query as a workunit. The 'Save stats to workunit.' option and 'Get summary stats.' option should not give to that roxie. @afishbeck please comment.

Copy link
Member

Choose a reason for hiding this comment

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

I asked because the code on line 348 will never be processed as it stands.

Also, is the traceLevel check-box still relevant?

Copy link
Member Author

@wangkx wangkx Oct 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The includeRoxieOptions will be set to 3 in the code of esp/services/ws_ecl/ws_ecl_service.cpp for a workunit roxie. In that case, the line 348 will be processed.

Also, is the traceLevel check-box still relevant?

I am not sure. @afishbeck please advise.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, when calling a published query you can tell Roxie the traceLevel to use. I’m not sure if there is an equivalent work unit parameter. As it is now that shouldn’t be exposed when submitting a work unit.

<tr>
<td class='input' align='left'>
<span>
Expand Down Expand Up @@ -339,6 +339,16 @@ function switchInputForm()
<option value="QUERY">Call Query</option>
<option value="WORKUNIT">Create Workunit</option>
</select>&nbsp;
</xsl:if>
<xsl:if test="$includeRoxieOptions=2">
<select id="job_type">
<option value="QUERY">Call Query</option>
</select>&nbsp;
</xsl:if>
<xsl:if test="$includeRoxieOptions=3">
<select id="job_type">
<option value="WORKUNIT">Create Workunit</option>
</select>&nbsp;
</xsl:if>
<select id="submit_type" name="submit_type">
<xsl:for-each select="/FormInfo/CustomViews/Result">
Expand Down
Loading