You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by yzang <gi...@git.apache.org> on 2017/06/25 04:39:34 UTC
[GitHub] bookkeeper pull request #210: BOOKKEEPER-1100: Add Http Server for Bookkeepe...
GitHub user yzang opened a pull request:
https://github.com/apache/bookkeeper/pull/210
BOOKKEEPER-1100: Add Http Server for Bookkeeper
BOOKKEEPER-1100:
Provide a general interface for HttpServer, which can be easily implemented by different HTTP frameworks.
This change include two implementations of HttpServer, one is TwitterServer and another one is Vertx.
Provide a general AbstractHandlerFactory, which is able to create handlers to handler different http apis,
and is able to be implemented for different HTTP frameworks.
Provide a service level classes, which include all the logics that http handlers will use.
Add test case for HttpServer
Descriptions of the changes in this PR:
xxxxxx
---
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
- [ ] Make sure the PR title is formatted like:
`<Issue #>: Description of pull request`
`e.g. Issue 123: Description ...`
- [ ] Make sure tests pass via `mvn clean apache-rat:check install findbugs:check`.
- [ ] Replace `<Issue #>` in the title with the actual Issue number, if there is one.
---
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yzang/bookkeeper yzang/http/yzang/BOOKKEEPER-1100
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/bookkeeper/pull/210.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #210
----
commit 4ba1bf02b43a8cbe6357ee3d9e655ffa76351571
Author: Yiming Zang <yz...@twitter.com>
Date: 2017-06-25T03:29:52Z
Add Http Server for Bookkeeper.
Provide a general interface for HttpServer, which can be easily implemented by different HTTP frameworks.
This change include two implementations of HttpServer, one is TwitterServer and another one is Vertx.
Provide a general AbstractHandlerFactory, which is able to create handlers to handler different http apis,
and is able to be implemented for different HTTP frameworks.
Provide a service level classes, which include all the logics that http handlers will use.
Add test case for HttpServer
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #210: BOOKKEEPER-1100: Add Http Server for Bookkeeper
Posted by jiazhai <gi...@git.apache.org>.
Github user jiazhai commented on the issue:
https://github.com/apache/bookkeeper/pull/210
@yzang, Would you please help change the last part of this PR description a little? If you did the checks, put an "x" in the "[ ]".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #210: BOOKKEEPER-1100: Add Http Server for Bookkeeper
Posted by jvrao <gi...@git.apache.org>.
Github user jvrao commented on the issue:
https://github.com/apache/bookkeeper/pull/210
We are adding REST endpoint to BookKeeper. I need to check if they overlap.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #210: BOOKKEEPER-1100: Add Http Server for Bookkeeper
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:
https://github.com/apache/bookkeeper/pull/210
@jvrao do you mind sending out your proposal. we might need to merge the efforts here, and also @eolivelli has some ideas on how to implement this with standard servlets.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #210: BOOKKEEPER-1100: Add Http Server for Bookkeeper
Posted by yzang <gi...@git.apache.org>.
Github user yzang commented on the issue:
https://github.com/apache/bookkeeper/pull/210
Found a few flaky/failing tests not related to this commit:
Running org.apache.bookkeeper.meta.TestLongZkLedgerIdGenerator
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 31.27 sec <<< FAILURE! - in org.apache.bookkeeper.meta.TestLongZkLedgerIdGenerator
testGenerateLedgerId(org.apache.bookkeeper.meta.TestLongZkLedgerIdGenerator) Time elapsed: 31.13 sec <<< FAILURE!
junit.framework.AssertionFailedError: Wait ledger id generation threads to stop timeout :
at org.apache.bookkeeper.meta.TestLongZkLedgerIdGenerator.testGenerateLedgerId(TestLongZkLedgerIdGenerator.java:130)
Running org.apache.bookkeeper.replication.AuditorLedgerCheckerTest
Tests run: 55, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 216.214 sec <<< FAILURE! - in org.apache.bookkeeper.replication.AuditorLedgerCheckerTest
testReadOnlyBookieShutdown[3](org.apache.bookkeeper.replication.AuditorLedgerCheckerTest) Time elapsed: 0.328 sec <<< FAILURE!
java.lang.AssertionError: latch should not have completed
at org.apache.bookkeeper.replication.AuditorLedgerCheckerTest.testReadOnlyBookieShutdown(AuditorLedgerCheckerTest.java:308)
Running org.apache.bookkeeper.client.TestFencing
Tests run: 7, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 65.266 sec <<< FAILURE! - in org.apache.bookkeeper.client.TestFencing
testManyOpenParallel(org.apache.bookkeeper.client.TestFencing) Time elapsed: 60.384 sec <<< ERROR!
org.junit.runners.model.TestTimedOutException: test timed out after 60000 milliseconds
at org.apache.bookkeeper.client.TestFencing.testManyOpenParallel(TestFencing.java:196)
Running org.apache.bookkeeper.test.BookieFailureTest
Tests run: 80, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 25.453 sec <<< FAILURE! - in org.apache.bookkeeper.test.BookieFailureTest
testLedgerOpenAfterBKCrashed[7](org.apache.bookkeeper.test.BookieFailureTest) Time elapsed: 0.352 sec <<< ERROR!
org.apache.bookkeeper.client.BKException$BKNotEnoughBookiesException
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #210: BOOKKEEPER-1100: Add Http Server for Bookkeeper
Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:
https://github.com/apache/bookkeeper/pull/210
Some comments:
1) is it possible NOT to bundle all the implementations in bookkeeper-server project ?
Or at least mark the dependencies as "provided" o "runtime" so that we do not make clients import transitively all that new jars. Maybe we can do as for stats-providers
2) we are giving access to Bookie configuration, what about security ? Maybe an user can provide a custom implementation of the HttpServer which requires auth ?
3) @reddycharan I think that this could be the base for implementing the dropped JMX-based features, we could (in another issue)
4) I think that this proposal does not 'force' a clear HTTP API to implementations, that is that from the various implementations it seems that we want to provide a REST-like API but it is really up to the implementations. This actually means that no one can write other tools to integrate with the Bookie, as the API will depend on the actual provider .
It would be better to decide a standard API and document it.
I am really in favor to make the low level implementation "pluggable": for instance I run my Bookies in the same process of a Tomcat and/or with Jetty.
I think that a better approach would be to provide a standard HttpServlet which could be deployed to any standard Web Container. This way we (BookKeeper community) will be driving the API and the security aspects of the Bookie
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] bookkeeper issue #210: BOOKKEEPER-1100: Add Http Server for Bookkeeper
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:
https://github.com/apache/bookkeeper/pull/210
regarding the servlet, I agree with @eolivelli , we probably should define a standard API and allow it running with different web container. @yzang : do finagle and vertx support servlet?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---