You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Daniel Barclay <db...@maprtech.com> on 2015/05/12 07:59:20 UTC
Review Request 34071: DRILL-3034: Apply UserException to port-binding;
handle in embedded-Drill case.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34071/
-----------------------------------------------------------
Review request for drill, abdelhakim deneche, Mehant Baid, and Parth Chandra.
Bugs: DRILL-3034
https://issues.apache.org/jira/browse/DRILL-3034
Repository: drill-git
Description
-------
Applied UserException to can't-bind-to-port error. [BasicServer]
Added specific handling of UserException (above case or other) in SQLException wrapping. [DrillConnectionImpl]
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java a148436
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 30279e6
Diff: https://reviews.apache.org/r/34071/diff/
Testing
-------
Manually tested in SQLLine and debugger.
Ran regular tests; no new failures.
Thanks,
Daniel Barclay
Re: Review Request 34071: DRILL-3034: Apply UserException to
port-binding; handle in embedded-Drill case.
Posted by Daniel Barclay <db...@maprtech.com>.
> On May 12, 2015, 3:26 p.m., abdelhakim deneche wrote:
> > exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java, line 101
> > <https://reviews.apache.org/r/34071/diff/1/?file=955966#file955966line101>
> >
> > this will alter the user exception's message. Sometimes this will clutter the original error message making it less useful to the user
I wanted to keep the "Failure in starting embedded Drillbit" text in both cases (whether the caught exception is a UserExcption or some other exception) for giving context for the lower-level error (to report what failed to be accomplished (starting Drill) because of the binding or other failure) and for consistency for usability.
> On May 12, 2015, 3:26 p.m., abdelhakim deneche wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java, line 204
> > <https://reviews.apache.org/r/34071/diff/1/?file=955965#file955965line204>
> >
> > build() already logs user exceptions at INFO level. Are you sure you want to log the same exception twice ?
No, I guess we don't need that.
Removed logging call.
- Daniel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34071/#review83408
-----------------------------------------------------------
On May 12, 2015, 5:59 a.m., Daniel Barclay wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34071/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 5:59 a.m.)
>
>
> Review request for drill, abdelhakim deneche, Mehant Baid, and Parth Chandra.
>
>
> Bugs: DRILL-3034
> https://issues.apache.org/jira/browse/DRILL-3034
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Applied UserException to can't-bind-to-port error. [BasicServer]
> Added specific handling of UserException (above case or other) in SQLException wrapping. [DrillConnectionImpl]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java a148436
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 30279e6
>
> Diff: https://reviews.apache.org/r/34071/diff/
>
>
> Testing
> -------
>
> Manually tested in SQLLine and debugger.
>
> Ran regular tests; no new failures.
>
>
> Thanks,
>
> Daniel Barclay
>
>
Re: Review Request 34071: DRILL-3034: Apply UserException to
port-binding; handle in embedded-Drill case.
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34071/#review83408
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java
<https://reviews.apache.org/r/34071/#comment134392>
build() already logs user exceptions at INFO level. Are you sure you want to log the same exception twice ?
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java
<https://reviews.apache.org/r/34071/#comment134393>
this will alter the user exception's message. Sometimes this will clutter the original error message making it less useful to the user
- abdelhakim deneche
On May 12, 2015, 5:59 a.m., Daniel Barclay wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34071/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 5:59 a.m.)
>
>
> Review request for drill, abdelhakim deneche, Mehant Baid, and Parth Chandra.
>
>
> Bugs: DRILL-3034
> https://issues.apache.org/jira/browse/DRILL-3034
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Applied UserException to can't-bind-to-port error. [BasicServer]
> Added specific handling of UserException (above case or other) in SQLException wrapping. [DrillConnectionImpl]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java a148436
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 30279e6
>
> Diff: https://reviews.apache.org/r/34071/diff/
>
>
> Testing
> -------
>
> Manually tested in SQLLine and debugger.
>
> Ran regular tests; no new failures.
>
>
> Thanks,
>
> Daniel Barclay
>
>
Re: Review Request 34071: DRILL-3034: Apply UserException to
port-binding; handle in embedded-Drill case.
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34071/#review83446
-----------------------------------------------------------
+1 (non binding)
- abdelhakim deneche
On May 12, 2015, 5:49 p.m., Daniel Barclay wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34071/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 5:49 p.m.)
>
>
> Review request for drill, abdelhakim deneche, Mehant Baid, and Parth Chandra.
>
>
> Bugs: DRILL-3034
> https://issues.apache.org/jira/browse/DRILL-3034
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Applied UserException to can't-bind-to-port error. [BasicServer]
> Added specific handling of UserException (above case or other) in SQLException wrapping. [DrillConnectionImpl]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java a148436
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 30279e6
>
> Diff: https://reviews.apache.org/r/34071/diff/
>
>
> Testing
> -------
>
> Manually tested in SQLLine and debugger.
>
> Ran regular tests; no new failures.
>
>
> Thanks,
>
> Daniel Barclay
>
>
Re: Review Request 34071: DRILL-3034: Apply UserException to
port-binding; handle in embedded-Drill case.
Posted by Daniel Barclay <db...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34071/
-----------------------------------------------------------
(Updated May 12, 2015, 5:49 p.m.)
Review request for drill, abdelhakim deneche, Mehant Baid, and Parth Chandra.
Changes
-------
Removed redundant logging call.
Bugs: DRILL-3034
https://issues.apache.org/jira/browse/DRILL-3034
Repository: drill-git
Description
-------
Applied UserException to can't-bind-to-port error. [BasicServer]
Added specific handling of UserException (above case or other) in SQLException wrapping. [DrillConnectionImpl]
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java a148436
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 30279e6
Diff: https://reviews.apache.org/r/34071/diff/
Testing
-------
Manually tested in SQLLine and debugger.
Ran regular tests; no new failures.
Thanks,
Daniel Barclay