Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
Cool, thanks @Randgalt, let me know when we decided the released schedule.
---
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
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
@Randgalt we haven't started the unifying progress yet, since we were
blocked by this feature, it would be better to publicize it on Curator's wiki
when we're actually using it inside FB, I'll
Github user Randgalt commented on the issue:
https://github.com/apache/curator/pull/165
@lvfangmin
> we're planning to unify the Zookeeper java client to be Curator inside
Facebook
Wow - cool! Can I publicize that on Curator's wiki?
> may I ask what's the
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
@Randgalt @cammckenzie Thanks for reviewing the code, we're planning to
unify the Zookeeper java client to be Curator inside Facebook, so it's really
important for us to track the client side
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
Yes, feel free to join remote, we'll post the ways to join online soon.
---
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
Github user Randgalt commented on the issue:
https://github.com/apache/curator/pull/165
Sounds like they'll have a remote option - I'm remote as well
---
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
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/165
Thanks @lvfangmin, unfortunately I'm somewhat geographically challenged
living on the other side of the world. Sounds like fun though.
---
If your project is set up for it, you can reply to
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
@Randgalt @cammckenzie, Facebook is going to hosted a Zookeeper Meetup,
there will be tech talks, swags, drinks and snacks, please join us if you're
interested in:
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
Hi @Randgalt, I've opened another PR for 3.0 branch, could you please take
a look? Let me know if there is any issue.
---
If your project is set up for it, you can reply to this email and have
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
Thanks @Randgalt, I've tried to merge this, there is not much conflict,
I'll send a separate PR based on CURATOR-3.0 today.
---
If your project is set up for it, you can reply to this email and
Github user Randgalt commented on the issue:
https://github.com/apache/curator/pull/165
I think we're almost there. Have you tried merging this with the
CURATOR-3.0 branch yet? Because this is a biggish change it would be good to
have a separate PR based off of CURATOR-3.0 to make
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
Hi @Randgalt, in case you're not aware, I've updated the code base are you
opinion, let me know if you have other comments, thanks.
---
If your project is set up for it, you can reply to this
Github user Randgalt commented on the issue:
https://github.com/apache/curator/pull/165
I'll try to get to this soon. It's a big-ish PR.
---
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
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
kindly ping.
---
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
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
Please take a look when you guys have time :)
---
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
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
@Randgalt I've updated the diff based what we discussed, please take a look
when you have time. And have a great weekends!
---
If your project is set up for it, you can reply to this email and
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
Thanks @Randgalt, I'll update the PR based on this.
---
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
Github user Randgalt commented on the issue:
https://github.com/apache/curator/pull/165
Yes - that should work. Create a new set of APIs but still support the old
API inside of the new one and mark the old one as deprecated.
---
If your project is set up for it, you can reply to
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
@Randgalt thanks for giving suggestions, there is another option: we create
a new tracer interface, like AdvancedTracerDriver which will expose more
metrics like we're doing in this PR. What's
Github user Randgalt commented on the issue:
https://github.com/apache/curator/pull/165
I haven't looked at the PR in depth. But, having this PR be backward
compatible should be explored. Possibly by emulating the old SPI/API.
---
If your project is set up for it, you can reply to
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
@Randgalt with new methods in the existing classes will also changes the
interface the users are using, it has the same side effect as this one, right?
---
If your project is set up for it, you
Github user Randgalt commented on the issue:
https://github.com/apache/curator/pull/165
I'm concerned that this change will force a major version bump from us.
Can't this be done with the existing classes and a few new methods?
---
If your project is set up for it, you can reply to
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
@cammckenzie Thanks again for reviewing the code, I think the millis is
good enough for tracing. I'll wait @Randgalt's review :)
---
If your project is set up for it, you can reply to this
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/165
I've had a bit more of a look and I think that the changes are OK. As you
mentioned, the changes are only to internal classes. There is a loss of
granularity (The TimeTrace was using nanos
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
ping.
---
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
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/165
Thanks @cammckenzie for review, the TimerTrace is actually an internal
helper class, it shouldn't have any backwards compatible problem. It is the
TracerDriver that may have backwards compatible
Github user cammckenzie commented on the issue:
https://github.com/apache/curator/pull/165
Thanks for the PR,
The content looks OK, but I think that the interface changes need to be
made backwards compatible. Can we change the OperationTrace to extend the
existing TimerTrace?
27 matches
Mail list logo
