You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Jarek Cecho <ja...@apache.org> on 2016/02/23 01:14:45 UTC

Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43857/
-----------------------------------------------------------

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2545
    https://issues.apache.org/jira/browse/SQOOP-2545


Repository: sqoop-sqoop2


Description
-------

I would like to see (&test) what happens when I call some random non-existing REST URLs to our server.


Diffs
-----

  server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java b894e37 
  server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java fb4a99f 
  test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/43857/diff/


Testing
-------


Thanks,

Jarek Cecho


Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

Posted by Jarek Cecho <ja...@apache.org>.

> On Feb. 23, 2016, 1:25 a.m., Colin Ma wrote:
> > server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java, line 145
> > <https://reviews.apache.org/r/43857/diff/1/?file=1264870#file1264870line145>
> >
> >     Is this for the authorization failed when request the /debug/* ?

Good question, I should have describe why I'm doing this change in the description. When I issue POST to my/cool/endpoint, then as this endpoint doesn't exists at all, it's handled by Jetty and Jetty returns 405. However if I execute POST to v1/connector (existing endpoint), then our code will return 500. I wanted to be consistent in handling that scenario and I've decided to change the 500 to 405 as I believe that it's better describing what's happening. Calling non existing endpoint is not really a "500 internal server error", it's a request error.


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43857/#review120234
-----------------------------------------------------------


On Feb. 23, 2016, 12:14 a.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2016, 12:14 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
>     https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I would like to see (&test) what happens when I call some random non-existing REST URLs to our server.
> 
> 
> Diffs
> -----
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java fb4a99f 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43857/#review120234
-----------------------------------------------------------




server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java (line 145)
<https://reviews.apache.org/r/43857/#comment181656>

    Is this for the authorization failed when request the /debug/* ?


- Colin Ma


On Feb. 23, 2016, 12:14 a.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2016, 12:14 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
>     https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I would like to see (&test) what happens when I call some random non-existing REST URLs to our server.
> 
> 
> Diffs
> -----
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java fb4a99f 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

Posted by Jarek Cecho <ja...@apache.org>.

> On March 1, 2016, 8:07 p.m., Abraham Fine wrote:
> > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java, line 48
> > <https://reviews.apache.org/r/43857/diff/2/?file=1268498#file1268498line48>
> >
> >     why is this change only being made for the submissionrequesthandler?
> >     
> >     don't we have something similar for the driverrequesthandler?

I'm trying to keep these patches rather small. I've did this change in Connector handler before and I will do them in DriverRequestHandler when I get to it. E.g. It's definitely in the scope of umbrella JIRA that I have.


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43857/#review121482
-----------------------------------------------------------


On Feb. 24, 2016, 11:26 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 11:26 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
>     https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I would like to see (&test) what happens when I call some random non-existing REST URLs to our server.
> 
> 
> Diffs
> -----
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java fb4a99f 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java 839a4d0 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java PRE-CREATION 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java e4c89df 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

Posted by Jarek Cecho <ja...@apache.org>.

> On March 1, 2016, 8:07 p.m., Abraham Fine wrote:
> > server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java, line 144
> > <https://reviews.apache.org/r/43857/diff/2/?file=1268499#file1268499line144>
> >
> >     perhaps there is a better way of handling this. i can imagine multiple different servererrors mapping to different http status codes.
> >     
> >     I would understand if you felt that this would be best suited for a different jira.

Yup, I would take that discussion to a separate JIRA - feel free to create one with your thoughts :)


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43857/#review121482
-----------------------------------------------------------


On Feb. 24, 2016, 11:26 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 11:26 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
>     https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I would like to see (&test) what happens when I call some random non-existing REST URLs to our server.
> 
> 
> Diffs
> -----
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java fb4a99f 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java 839a4d0 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java PRE-CREATION 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java e4c89df 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

Posted by Abraham Fine <ab...@abrahamfine.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43857/#review121482
-----------------------------------------------------------




server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 
<https://reviews.apache.org/r/43857/#comment183197>

    why is this change only being made for the submissionrequesthandler?
    
    don't we have something similar for the driverrequesthandler?



server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java (line 144)
<https://reviews.apache.org/r/43857/#comment183196>

    perhaps there is a better way of handling this. i can imagine multiple different servererrors mapping to different http status codes.
    
    I would understand if you felt that this would be best suited for a different jira.


- Abraham Fine


On Feb. 24, 2016, 11:26 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 11:26 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
>     https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I would like to see (&test) what happens when I call some random non-existing REST URLs to our server.
> 
> 
> Diffs
> -----
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java fb4a99f 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java 839a4d0 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java PRE-CREATION 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java e4c89df 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

Posted by Jarek Cecho <ja...@apache.org>.

> On March 4, 2016, 12:29 a.m., Jarek Cecho wrote:
> > server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java, line 144
> > <https://reviews.apache.org/r/43857/diff/2/?file=1268499#file1268499line144>
> >
> >     Yup, I would take that discussion to a separate JIRA - feel free to create one with your thoughts :)

Ignore this comment please.


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43857/#review121970
-----------------------------------------------------------


On Feb. 24, 2016, 11:26 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 11:26 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
>     https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I would like to see (&test) what happens when I call some random non-existing REST URLs to our server.
> 
> 
> Diffs
> -----
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java fb4a99f 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java 839a4d0 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java PRE-CREATION 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java e4c89df 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43857/#review121970
-----------------------------------------------------------




server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java (line 144)
<https://reviews.apache.org/r/43857/#comment183826>

    Yup, I would take that discussion to a separate JIRA - feel free to create one with your thoughts :)


- Jarek Cecho


On Feb. 24, 2016, 11:26 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 11:26 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
>     https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> I would like to see (&test) what happens when I call some random non-existing REST URLs to our server.
> 
> 
> Diffs
> -----
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java fb4a99f 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java 839a4d0 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java PRE-CREATION 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java e4c89df 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43857/
-----------------------------------------------------------

(Updated Feb. 24, 2016, 11:26 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2545
    https://issues.apache.org/jira/browse/SQOOP-2545


Repository: sqoop-sqoop2


Description
-------

I would like to see (&test) what happens when I call some random non-existing REST URLs to our server.


Diffs (updated)
-----

  server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java b894e37 
  server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java fb4a99f 
  test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java 839a4d0 
  test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java PRE-CREATION 
  test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java e4c89df 

Diff: https://reviews.apache.org/r/43857/diff/


Testing
-------


Thanks,

Jarek Cecho