+1
On Tue, Sep 19, 2017 at 6:44 PM, Yiming Zang <yz...@twitter.com.invalid> wrote: > LGTM +1 > > On Tue, Sep 19, 2017 at 8:26 AM, Enrico Olivelli <eolive...@gmail.com> > wrote: > >> Looks good to me. >> Enrico >> >> On mar 19 set 2017, 17:22 Jia Zhai <zhaiji...@gmail.com> wrote: >> >> > Any thoughts or comments on this. :) >> > If not, would like to mark this BP approved. And we will prepare an >> > initial PR for detailed discussion and comments. >> > >> > On Wed, Sep 13, 2017 at 4:49 PM, Sijie Guo <guosi...@gmail.com> wrote: >> > >> >> On Wed, Sep 13, 2017 at 1:38 AM, Enrico Olivelli <eolive...@gmail.com> >> >> wrote: >> >> >> >> > >> >> > >> >> > 2017-09-13 10:31 GMT+02:00 Jia Zhai <zhaiji...@gmail.com>: >> >> > >> >> >> Thanks a lot for your time Yiming and Enrico. :) >> >> >> >> >> >> Regarding the security, we could do it in a separate BP, and make >> >> this BP >> >> >> more focus on filling up the useful endpoints. How about it? >> >> >> >> >> > >> >> > OK, but please consider that the more utility you add to the endpoint >> >> the >> >> > more users will want to enable it and so security will be a blocker >> >> issue >> >> > >> >> >> >> My take on this - we all know security is important for any >> communication >> >> to bookies. >> >> >> >> However, security is a big different scope of problems to address. we >> >> shouldn't put everything in one big BP. I'd suggest us focusing on the >> >> problems >> >> that a BP tends to address, defer other things to separate BPs. >> Otherwise >> >> we can't land any changes quickly. >> >> >> >> >> >> > >> >> > >> >> >> >> >> >> On Wed, Sep 13, 2017 at 3:30 PM, Enrico Olivelli < >> eolive...@gmail.com> >> >> >> wrote: >> >> >> >> >> >> > Hi Jia, >> >> >> > I am OK with the idea of having standard HTTP API, this will help >> >> >> > development of (non-Java) tools. >> >> >> > >> >> >> > It is not clear to me if we are going to add an http API useful for >> >> >> > "managing bookies" or a new HTTP REST-like Client API to >> BookKeeper. >> >> >> > I am referring to the fact that in the proposal there are API calls >> >> to >> >> >> > create ledgers and to read data. >> >> >> > I think we should separate this two aspects and maybe it is better >> to >> >> >> > address the 'bookie management' first, which is the work that >> Yiming >> >> >> > started. >> >> >> > >> >> >> [jia] It is targeting for the admin portal. the existence of the >> ledger >> >> >> api >> >> >> is just to simplify debugging or operations. we can eliminate the >> >> ‘create’ >> >> >> endpoint. >> >> >> >> >> > >> >> > OK so are you already thinking about specific tools to manage bookies >> >> > without the bookie shell >> >> > >> >> >> >> We would like to integrate this for schedulers (e.g. k8s). so we can >> >> deploy >> >> and operate bookkeeper easily >> >> in those environments. >> >> >> >> >> >> > >> >> > >> >> >> >> >> >> >> >> >> > >> >> >> > Another point very important to me is that if we are going to >> >> introduce >> >> >> > management operations via http we have to take into account >> >> security, at >> >> >> > least TLS and some kind of authentication. >> >> >> > About TLS it is very simple to achieve this, every HTTP server >> >> supports >> >> >> TLS >> >> >> > (https) >> >> >> > About authentication the problem is not so simple, Kerberos on >> HTTPs >> >> is >> >> >> > very complex, but we need to introduce some auth mechanism. >> >> >> > IMHO We can require the 'http server implementation' to implement >> >> >> security >> >> >> > but out of the box we have to supply basic support at least for one >> >> >> > provider bundled in the distribution package. >> >> >> > >> >> >> [jia] The BP focuses on filling up the useful endpoints. The security >> >> will >> >> >> be a separate BP. >> >> >> >> >> >> >> >> >> > >> >> >> > Some API can return very large result sets like 'list ledgers', >> >> actually >> >> >> > the HTTP Server subsystem exchanges strings in memory, we will need >> >> to >> >> >> > introduce some more smart way because it would be easy to bring >> down >> >> the >> >> >> > bookie just by calling that API multiple times concurrencly (it is >> >> just >> >> >> an >> >> >> > example) >> >> >> > >> >> >> [jia] We could add pagination into all the `list` api. >> >> >> >> >> > >> >> > Yes but it will be really tricky to implement, but please do it >> >> > >> >> > >> >> >> >> >> >> > >> >> >> > IMHO The 'create ledger' API is not useful, due to the design of >> BK >> >> you >> >> >> > have to create a ledger and then write immediately to it, I think >> >> that >> >> >> such >> >> >> > an API should allow the client to stream the contents of the ledger >> >> in >> >> >> the >> >> >> > HTTP body at least. But I think that a more stateful http API needs >> >> to >> >> >> be >> >> >> > designed to implement a pure Http client >> >> >> > >> >> >> [jia] Thanks, we are not planning to implement an http client. we >> will >> >> >> remove ‘create’ here. >> >> >> >> >> > >> >> > OK, thanks >> >> > >> >> >> >> >> >> >> >> >> > >> >> >> > Maybe we should add a more narrowed motivation and add the only >> >> useful >> >> >> APIs >> >> >> > to address those issues. >> >> >> > For instance in my company we would like to start creating a >> >> BookKeeper >> >> >> Web >> >> >> > UI, so we need some way to talk to bookies and exchange data, but >> in >> >> >> this >> >> >> > case I am interested in asking to bookies their status and the >> >> effective >> >> >> > contents of the bookie >> >> >> > >> >> >> [jia] The BP proposes adding a standard naming convention for adding >> >> admin >> >> >> endpoints. Feel free to propose the endpoints you would like to >> appear >> >> in >> >> >> the http admin portal. >> >> >> >> >> > >> >> > The ones you wrote are enough complete for me, I would like to have >> >> > read-only operation in order to have a global view on the status of >> the >> >> > system. >> >> > >> >> > -- Enrico >> >> > >> >> > >> >> > >> >> >> >> >> >> >> >> >> > >> >> >> > -- Enrico >> >> >> > >> >> >> > >> >> >> > 2017-09-13 6:09 GMT+02:00 Yiming Zang <yz...@twitter.com.invalid>: >> >> >> > >> >> >> > > Sure, I think the current HTTP endpoints in Twitter are only >> >> designed >> >> >> for >> >> >> > > Twitter specific, such as check quorum loss, check rack/region >> >> >> diversity. >> >> >> > > So the endpoints convention in Twitter are not the same as in the >> >> >> > proposal. >> >> >> > > I think it would be great to have an agreement on the API naming >> >> >> design, >> >> >> > so >> >> >> > > I like the API design in the proposal, I think the proposal looks >> >> >> good to >> >> >> > > me. >> >> >> > > >> >> >> > > Besides, we're currently only using GET functionalities in >> Twitter, >> >> >> but I >> >> >> > > notice there're a lot of POST and PUT APIs in the proposal which >> >> could >> >> >> > > change the bookie state or trigger some heavy workload. These >> APIs >> >> >> looks >> >> >> > a >> >> >> > > bit risky to me if we don't have any authentication enabled (in >> >> >> Twitter). >> >> >> > > >> >> >> > > On Tue, Sep 12, 2017 at 6:11 PM, Sijie Guo <guosi...@gmail.com> >> >> >> wrote: >> >> >> > > >> >> >> > > > + Yiming >> >> >> > > > >> >> >> > > > Yiming, if you have time, please take a look at this BP. let's >> >> see >> >> >> if >> >> >> > > > there are any conflicts with those you added for autorecovery. >> >> >> > > > >> >> >> > > > - Sijie >> >> >> > > > >> >> >> > > > On Tue, Sep 12, 2017 at 8:00 AM, Jia Zhai <zhaiji...@gmail.com >> > >> >> >> wrote: >> >> >> > > > >> >> >> > > >> Hi all, >> >> >> > > >> >> >> >> > > >> Based on Github #278 <https://github.com/apache/boo >> >> >> kkeeper/pull/278>, >> >> >> > > I >> >> >> > > >> have just posted a proposal regarding define BookKeeper public >> >> http >> >> >> > > >> endpoints: >> >> >> > > >> >> >> >> > > >> https://cwiki.apache.org/confluence/display/BOOKKEEPER/BP- >> >> >> > > >> 17%3A+Define+BookKeeper+public+http+endpoints >> >> >> > > >> >> >> >> > > >> >> >> >> > > >> Github #278 <https://github.com/apache/bookkeeper/pull/278> >> >> >> > introduces >> >> >> > > >> BookKeeper Http Endpoint module. However there are only two >> >> >> endpoints, >> >> >> > > >> which is “/heartbeat” and “/api/config/serverconfig”, defined >> in >> >> >> #278. >> >> >> > > In >> >> >> > > >> order to fully leverage the http modules, The goal of this BP >> >> is to >> >> >> > add >> >> >> > > >> more endpoints to this modules. >> >> >> > > >> >> >> >> > > >> Any comments are welcome and appreciated. >> >> >> > > >> >> >> >> > > >> >> >> >> > > >> Thanks. >> >> >> > > >> >> >> >> > > >> -Jia >> >> >> > > >> >> >> >> > > > >> >> >> > > > >> >> >> > > >> >> >> > >> >> >> >> >> > >> >> > >> >> >> > >> > -- >> >> >> -- Enrico Olivelli >>
