<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Sat, 15 Aug 2015 at 01:01 Brian Stansberry <<a href="mailto:brian.stansberry@redhat.com">brian.stansberry@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Looks good.<br>
<br>
Do we want migration-warnings=-[] in the success case, or just omit it?<br>
I don't have a strong preference either way.<br></blockquote><div><br></div><div>I have changed it so this will not be displayed. </div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Seeing all the details in the failure case made me nervous about RBAC.<br>
But I think the key there is in the read of the subsystem that goes to<br>
produce the ops. We should test that that fails if the caller is not<br>
able to address some child resource or if the caller is not able to read<br>
some attribute value.<br></blockquote><div><br></div><div>Should this hold up the PR, or is this a task for later?</div><div><br></div><div>Stuart</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
If we know the caller can read the entire subsystem then it is safe to<br>
provide details of any failure on the writes, since it is the same data.<br>
<br>
Once everything is clear we should work out a way to have shared code so<br>
this stuff has a higher guarantee of consistency across subsystems.<br>
<br>
On 8/14/15 1:32 AM, Stuart Douglas wrote:<br>
> I have a PR that builds on Brian's work for the web migrate op:<br>
> <a href="https://github.com/wildfly/wildfly/pull/7931" rel="noreferrer" target="_blank">https://github.com/wildfly/wildfly/pull/7931</a><br>
><br>
> Output is:<br>
><br>
> Standard EAP config (no warnings or errors):<br>
><br>
> [standalone@localhost:9999 /] /subsystem=web:migrate<br>
> {<br>
> "outcome" => "success",<br>
> "result" => {"migration-warnings" => []}<br>
> }<br>
><br>
> Success with a warning (in this case due to an invalid protocol<br>
> attribute on a connector):<br>
><br>
> [standalone@localhost:9999 /] /subsystem=web:migrate<br>
> {<br>
> "outcome" => "success",<br>
> "result" => {"migration-warnings" => ["WFLYWEB0002: Could not<br>
> migrate resource {<br>
> \"protocol\" => \"INVALID\",<br>
> \"scheme\" => \"http\",<br>
> \"socket-binding\" => \"ajp\",<br>
> \"enable-lookups\" => undefined,<br>
> \"proxy-binding\" => undefined,<br>
> \"proxy-name\" => undefined,<br>
> \"proxy-port\" => undefined,<br>
> \"redirect-binding\" => undefined,<br>
> \"redirect-port\" => undefined,<br>
> \"secure\" => undefined,<br>
> \"max-post-size\" => undefined,<br>
> \"max-save-post-size\" => undefined,<br>
> \"enabled\" => undefined,<br>
> \"executor\" => undefined,<br>
> \"max-connections\" => undefined,<br>
> \"virtual-server\" => undefined,<br>
> \"name\" => \"ajp\",<br>
> \"operation\" => \"add\",<br>
> \"address\" => [<br>
> (\"subsystem\" => \"web\"),<br>
> (\"connector\" => \"ajp\")<br>
> ]<br>
> }"]}<br>
> }<br>
><br>
> Failed OP:<br>
><br>
> [standalone@localhost:9999 /] /subsystem=web:migrate<br>
> {<br>
> "outcome" => "failed",<br>
> "result" => {<br>
> "migration-warnings" => [],<br>
> "migration-error" => {<br>
> "operation" => {<br>
> "operation" => "add",<br>
> "address" => [<br>
> ("subsystem" => "undertow"),<br>
> ("server" => "default-server"),<br>
> ("ajp-listener" => "ajp")<br>
> ],<br>
> "socket-binding" => "ajp",<br>
> "secure" => undefined,<br>
> "redirect-socket" => undefined,<br>
> "enabled" => undefined,<br>
> "resolve-peer-address" => undefined,<br>
> "max-post-size" => undefined,<br>
> "max-connections" => "sadfadsfasdgasdfg",<br>
> "operation-headers" => {<br>
> "caller-type" => "user",<br>
> "access-mechanism" => "NATIVE"<br>
> },<br>
> "worker" => undefined,<br>
> "buffer-pool" => undefined,<br>
> "disallowed-methods" => undefined,<br>
> "max-header-size" => undefined,<br>
> "buffer-pipelined-data" => undefined,<br>
> "max-parameters" => undefined,<br>
> "max-headers" => undefined,<br>
> "max-cookies" => undefined,<br>
> "allow-encoded-slash" => undefined,<br>
> "decode-url" => undefined,<br>
> "url-charset" => undefined,<br>
> "always-set-keep-alive" => undefined,<br>
> "max-buffered-request-size" => undefined,<br>
> "record-request-start-time" => undefined,<br>
> "allow-equals-in-cookie-value" => undefined,<br>
> "no-request-timeout" => undefined,<br>
> "request-parse-timeout" => undefined,<br>
> "tcp-backlog" => undefined,<br>
> "receive-buffer" => undefined,<br>
> "send-buffer" => undefined,<br>
> "tcp-keep-alive" => undefined,<br>
> "read-timeout" => undefined,<br>
> "write-timeout" => undefined<br>
> },<br>
> "result" => {<br>
> "outcome" => "failed",<br>
> "failure-description" => "WFLYCTL0097: Wrong type for<br>
> max-connections. Expected [INT] but was STRING",<br>
> "rolled-back" => true<br>
> }<br>
> }<br>
> },<br>
> "rolled-back" => true<br>
> }<br>
><br>
> Stuart<br>
><br>
><br>
><br>
><br>
> On Fri, 14 Aug 2015 at 11:03 Brian Stansberry<br>
> <<a href="mailto:brian.stansberry@redhat.com" target="_blank">brian.stansberry@redhat.com</a> <mailto:<a href="mailto:brian.stansberry@redhat.com" target="_blank">brian.stansberry@redhat.com</a>>> wrote:<br>
><br>
> On 8/13/15 10:03 AM, Brian Stansberry wrote:<br>
> > On 8/13/15 3:11 AM, Stuart Douglas wrote:<br>
> >><br>
> >><br>
> >> On Thu, 13 Aug 2015 at 16:27 Ladislav Thon <<a href="mailto:lthon@redhat.com" target="_blank">lthon@redhat.com</a><br>
> <mailto:<a href="mailto:lthon@redhat.com" target="_blank">lthon@redhat.com</a>><br>
> >> <mailto:<a href="mailto:lthon@redhat.com" target="_blank">lthon@redhat.com</a> <mailto:<a href="mailto:lthon@redhat.com" target="_blank">lthon@redhat.com</a>>>> wrote:<br>
> >><br>
> >><br>
> >> > The approach I took was to add the warnings to the<br>
> describe-migration<br>
> >> > op, and when running the migrate op make the<br>
> describe-migration<br>
> >> > operation the first step in the composite, so the output<br>
> looks like<br>
> >> > this: <a href="http://pastebin.com/B01KNAHX" rel="noreferrer" target="_blank">http://pastebin.com/B01KNAHX</a><br>
> >><br>
> >> The fact that it's a composite operation makes the output a<br>
> bit cryptic<br>
> >> (step-X?), but otherwise I think it's actually good.<br>
> >><br>
> >> I guess that only handling the warnings in<br>
> :describe-migration makes the<br>
> >> implementation easier (not sure?) and printing<br>
> :describe-migration as a<br>
> >> part of :migrate makes sense too, precisely because it<br>
> shows the steps<br>
> >> that are performed.<br>
> >><br>
> >> So my only objection would be those cryptic "step-X" names,<br>
> but I guess<br>
> >> that we can live with that (if all :migrate operations took<br>
> the same<br>
> >> approach; inconsistency wouldn't really help here).<br>
> >><br>
> >><br>
> >> That is because this is implemented as a composite operation, AFAIK<br>
> >> there is lots of special handing for composite ops, so I don't<br>
> think we<br>
> >> want to be implementing our own one with different output just<br>
> for this<br>
> >> case.<br>
> >><br>
> >> I could be wrong though, this is not really my area, Brian will<br>
> know more.<br>
> >><br>
> ><br>
> > In the success case, I don't see the point of outputting "step-2" =><br>
> > {"outcome" => "success"}, "step-3" => {"outcome" => "success"}, etc.<br>
> ><br>
> > In the failure case though, having an intelligible<br>
> failure-description<br>
> > is important. And the composite op handler may be getting in your<br>
> way there<br>
> ><br>
> > Instead of writing more text, I'll write some code to illustrate some<br>
> > thoughts and post back later.<br>
> ><br>
><br>
> Here's what I was thinking.<br>
><br>
> [1] is a change to break out the step registration stuff<br>
> CompositeOperationHandler does into a utility method, with a couple<br>
> other variants added that are better suited for use by other OSHs.<br>
><br>
> Basically, it lets you pass in a Map of ops and a holder map of response<br>
> nodes, and it will add the steps and populate the response node map.<br>
> (You can optionally pre-populate the response node map. There's also a<br>
> variant that uses lists instead of maps in case that's more convenient.)<br>
><br>
> Using that, the response nodes for the added ops don't have to directly<br>
> appear in the overall operation response. Instead the migrate op handler<br>
> can hold a ref to them and in its ResultHandler use them to format an<br>
> understandable response.<br>
><br>
> The top two commits in [2] are an example tweak to the current<br>
> WebMigrationOp in master to illustrate use of this.<br>
><br>
> [1] <a href="https://github.com/wildfly/wildfly-core/pull/956" rel="noreferrer" target="_blank">https://github.com/wildfly/wildfly-core/pull/956</a><br>
><br>
> [2] <a href="https://github.com/bstansberry/wildfly/tree/help-migrate-multistep" rel="noreferrer" target="_blank">https://github.com/bstansberry/wildfly/tree/help-migrate-multistep</a><br>
><br>
> >> Stuart<br>
> >><br>
> >><br>
> >> LT<br>
> >> _______________________________________________<br>
> >> wildfly-dev mailing list<br>
> >> <a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a> <mailto:<a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a>><br>
> <mailto:<a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a><br>
> <mailto:<a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a>>><br>
> >> <a href="https://lists.jboss.org/mailman/listinfo/wildfly-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/wildfly-dev</a><br>
> >><br>
> >><br>
> >><br>
> >> _______________________________________________<br>
> >> wildfly-dev mailing list<br>
> >> <a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a> <mailto:<a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a>><br>
> >> <a href="https://lists.jboss.org/mailman/listinfo/wildfly-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/wildfly-dev</a><br>
> >><br>
> ><br>
> ><br>
><br>
><br>
> --<br>
> Brian Stansberry<br>
> Senior Principal Software Engineer<br>
> JBoss by Red Hat<br>
> _______________________________________________<br>
> wildfly-dev mailing list<br>
> <a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a> <mailto:<a href="mailto:wildfly-dev@lists.jboss.org" target="_blank">wildfly-dev@lists.jboss.org</a>><br>
> <a href="https://lists.jboss.org/mailman/listinfo/wildfly-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/wildfly-dev</a><br>
><br>
<br>
<br>
--<br>
Brian Stansberry<br>
Senior Principal Software Engineer<br>
JBoss by Red Hat<br>
</blockquote></div></div>