You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Li Li <li...@cloudera.com> on 2016/02/11 02:04:11 UTC
Review Request 43464: SENTRY-520: Use the twitter Bootstrap kit (or
similar) to beautify the Sentry Service webpage
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43464/
-----------------------------------------------------------
Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
Use the twitter Bootstrap kit (or similar) to beautify the Sentry Service webpage
Diffs
-----
sentry-provider/sentry-provider-db/pom.xml 38e0924f18383b3f0186e4810db3e1a057c2f6b6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryServiceServlet.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java fdb99ce091d59fc475f35c4cc263975cb909120e
sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html PRE-CREATION
sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap-theme.min.css PRE-CREATION
sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap.min.css PRE-CREATION
sentry-provider/sentry-provider-db/src/main/webapp/css/sentry.css PRE-CREATION
sentry-provider/sentry-provider-db/src/main/webapp/sentry.png PRE-CREATION
Diff: https://reviews.apache.org/r/43464/diff/
Testing
-------
The prejenkins job has some issue with the png file (sentry logo) in the patch, so I removed the sentry logo when submiting the patch there.
Thanks,
Li Li
Re: Review Request 43464: SENTRY-520: Use the twitter Bootstrap kit
(or similar) to beautify the Sentry Service webpage
Posted by Li Li <li...@cloudera.com>.
> On Feb. 18, 2016, 6:50 p.m., Lenni Kuff wrote:
> > I think this patch is missing the SentryServiceServlet.java file
I just removed SentryServiceServlet.java class, and reuse the original AdminServlet.class in codahale lib. Their content is similar, and only works for codahale stuff.
- Li
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43464/#review119670
-----------------------------------------------------------
On Feb. 18, 2016, 6:34 p.m., Li Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43464/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 6:34 p.m.)
>
>
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Use the twitter Bootstrap kit (or similar) to beautify the Sentry Service webpage
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 38e0924f18383b3f0186e4810db3e1a057c2f6b6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java fdb99ce091d59fc475f35c4cc263975cb909120e
> sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap-theme.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/sentry.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/sentry.png PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43464/diff/
>
>
> Testing
> -------
>
> The prejenkins job has some issue with the png file (sentry logo) in the patch, so I removed the sentry logo when submiting the patch there.
>
>
> Thanks,
>
> Li Li
>
>
Re: Review Request 43464: SENTRY-520: Use the twitter Bootstrap kit
(or similar) to beautify the Sentry Service webpage
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43464/#review119670
-----------------------------------------------------------
I think this patch is missing the SentryServiceServlet.java file
- Lenni Kuff
On Feb. 18, 2016, 6:34 p.m., Li Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43464/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 6:34 p.m.)
>
>
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Use the twitter Bootstrap kit (or similar) to beautify the Sentry Service webpage
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 38e0924f18383b3f0186e4810db3e1a057c2f6b6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java fdb99ce091d59fc475f35c4cc263975cb909120e
> sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap-theme.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/sentry.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/sentry.png PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43464/diff/
>
>
> Testing
> -------
>
> The prejenkins job has some issue with the png file (sentry logo) in the patch, so I removed the sentry logo when submiting the patch there.
>
>
> Thanks,
>
> Li Li
>
>
Re: Review Request 43464: SENTRY-520: Use the twitter Bootstrap kit
(or similar) to beautify the Sentry Service webpage
Posted by Li Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43464/#review120055
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html (line 18)
<https://reviews.apache.org/r/43464/#comment181429>
It will be loaded automatically as it is already set in resourceHandler.setWelcomeFiles().
- Li Li
On Feb. 18, 2016, 6:34 p.m., Li Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43464/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 6:34 p.m.)
>
>
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Use the twitter Bootstrap kit (or similar) to beautify the Sentry Service webpage
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 38e0924f18383b3f0186e4810db3e1a057c2f6b6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java fdb99ce091d59fc475f35c4cc263975cb909120e
> sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap-theme.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/sentry.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/sentry.png PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43464/diff/
>
>
> Testing
> -------
>
> The prejenkins job has some issue with the png file (sentry logo) in the patch, so I removed the sentry logo when submiting the patch there.
>
>
> Thanks,
>
> Li Li
>
>
Re: Review Request 43464: SENTRY-520: Use the twitter Bootstrap kit
(or similar) to beautify the Sentry Service webpage
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43464/#review120054
-----------------------------------------------------------
Fix it, then Ship it!
sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html (line 18)
<https://reviews.apache.org/r/43464/#comment181428>
Should this file be index.xml so it is loaded by default, or does that happen automatically?
- Lenni Kuff
On Feb. 18, 2016, 6:34 p.m., Li Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43464/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 6:34 p.m.)
>
>
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Use the twitter Bootstrap kit (or similar) to beautify the Sentry Service webpage
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 38e0924f18383b3f0186e4810db3e1a057c2f6b6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java fdb99ce091d59fc475f35c4cc263975cb909120e
> sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap-theme.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/sentry.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/sentry.png PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43464/diff/
>
>
> Testing
> -------
>
> The prejenkins job has some issue with the png file (sentry logo) in the patch, so I removed the sentry logo when submiting the patch there.
>
>
> Thanks,
>
> Li Li
>
>
Re: Review Request 43464: SENTRY-520: Use the twitter Bootstrap kit
(or similar) to beautify the Sentry Service webpage
Posted by Li Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43464/
-----------------------------------------------------------
(Updated Feb. 18, 2016, 6:34 p.m.)
Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
Changes
-------
Remove SentryServiceServlet, and reuse original AdminServlet.
Repository: sentry
Description
-------
Use the twitter Bootstrap kit (or similar) to beautify the Sentry Service webpage
Diffs (updated)
-----
sentry-provider/sentry-provider-db/pom.xml 38e0924f18383b3f0186e4810db3e1a057c2f6b6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java fdb99ce091d59fc475f35c4cc263975cb909120e
sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html PRE-CREATION
sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap-theme.min.css PRE-CREATION
sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap.min.css PRE-CREATION
sentry-provider/sentry-provider-db/src/main/webapp/css/sentry.css PRE-CREATION
sentry-provider/sentry-provider-db/src/main/webapp/sentry.png PRE-CREATION
Diff: https://reviews.apache.org/r/43464/diff/
Testing
-------
The prejenkins job has some issue with the png file (sentry logo) in the patch, so I removed the sentry logo when submiting the patch there.
Thanks,
Li Li
Re: Review Request 43464: SENTRY-520: Use the twitter Bootstrap kit
(or similar) to beautify the Sentry Service webpage
Posted by Li Li <li...@cloudera.com>.
> On Feb. 18, 2016, 1:42 a.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java, line 79
> > <https://reviews.apache.org/r/43464/diff/1/?file=1240069#file1240069line79>
> >
> > Use LOGGER.debug("<message>", e); it will handle unrolling and printing the stack trace. Also, should this be LOGGER.error?
fixed.
> On Feb. 18, 2016, 1:42 a.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryServiceServlet.java, line 33
> > <https://reviews.apache.org/r/43464/diff/1/?file=1240068#file1240068line33>
> >
> > Add class comment
fixed
> On Feb. 18, 2016, 1:42 a.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryServiceServlet.java, line 34
> > <https://reviews.apache.org/r/43464/diff/1/?file=1240068#file1240068line34>
> >
> > What is this healthchecks page? Also, what is the "ping" page?
Those are all from codehale metrics. HealthCheckServlet responds to GET requests by running all the health check. We have a SentryHealthCheckServletContextListener class. Though we don't have any health check yet, it is better to keep it for the future use. PingServlet is used for alive checks.
> On Feb. 18, 2016, 1:42 a.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryServiceServlet.java, line 39
> > <https://reviews.apache.org/r/43464/diff/1/?file=1240068#file1240068line39>
> >
> > Explain why these are tagged transient
Use 'transient' to reduce serialization overhead
- Li
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43464/#review118822
-----------------------------------------------------------
On Feb. 11, 2016, 1:04 a.m., Li Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43464/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 1:04 a.m.)
>
>
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Use the twitter Bootstrap kit (or similar) to beautify the Sentry Service webpage
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 38e0924f18383b3f0186e4810db3e1a057c2f6b6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryServiceServlet.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java fdb99ce091d59fc475f35c4cc263975cb909120e
> sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap-theme.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/sentry.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/sentry.png PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43464/diff/
>
>
> Testing
> -------
>
> The prejenkins job has some issue with the png file (sentry logo) in the patch, so I removed the sentry logo when submiting the patch there.
>
>
> Thanks,
>
> Li Li
>
>
Re: Review Request 43464: SENTRY-520: Use the twitter Bootstrap kit
(or similar) to beautify the Sentry Service webpage
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43464/#review118822
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryServiceServlet.java (line 33)
<https://reviews.apache.org/r/43464/#comment180129>
Add class comment
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryServiceServlet.java (line 34)
<https://reviews.apache.org/r/43464/#comment180916>
What is this healthchecks page? Also, what is the "ping" page?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryServiceServlet.java (line 39)
<https://reviews.apache.org/r/43464/#comment180915>
Explain why these are tagged transient
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java (line 78)
<https://reviews.apache.org/r/43464/#comment180130>
Use LOGGER.debug("<message>", e); it will handle unrolling and printing the stack trace. Also, should this be LOGGER.error?
- Lenni Kuff
On Feb. 11, 2016, 1:04 a.m., Li Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43464/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 1:04 a.m.)
>
>
> Review request for sentry, Anne Yu, Hao Hao, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Use the twitter Bootstrap kit (or similar) to beautify the Sentry Service webpage
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 38e0924f18383b3f0186e4810db3e1a057c2f6b6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryServiceServlet.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java fdb99ce091d59fc475f35c4cc263975cb909120e
> sentry-provider/sentry-provider-db/src/main/webapp/SentryService.html PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap-theme.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/bootstrap.min.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/css/sentry.css PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/webapp/sentry.png PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43464/diff/
>
>
> Testing
> -------
>
> The prejenkins job has some issue with the png file (sentry logo) in the patch, so I removed the sentry logo when submiting the patch there.
>
>
> Thanks,
>
> Li Li
>
>