You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@curator.apache.org by iocanel <gi...@git.apache.org> on 2014/11/06 18:55:25 UTC
[GitHub] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
GitHub user iocanel opened a pull request:
https://github.com/apache/curator/pull/53
[CURATOR-160] Add builders and dsl for ZooKeeper's config and reconfig m...
...ethods.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/iocanel/curator CURATOR-160
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/curator/pull/53.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 #53
----
commit 2c576dc344a167ad4a72d71412c98d76ff4e2d3d
Author: Ioannis Canellos <io...@gmail.com>
Date: 2014-11-06T15:34:47Z
[CURATOR-160] Add builders and dsl for ZooKeeper's config and reconfig methods.
----
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by iocanel <gi...@git.apache.org>.
Github user iocanel commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-90658537
Yeah,
I think that I’ll have it in by the end of the week.
> On Apr 7, 2015, at 8:22 PM, Andrew Kondratovich <no...@github.com> wrote:
>
> Hi, any update on this?
>
> —
> Reply to this email directly or view it on GitHub <https://github.com/apache/curator/pull/53#issuecomment-90653266>.
>
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by Randgalt <gi...@git.apache.org>.
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/53#discussion_r20757865
--- Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java ---
@@ -0,0 +1,29 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.curator.framework.api;
+
+public interface Configurable<T> {
+
+ /**
+ * Sets the configuration version to use.
+ * @param config The version of the configuration.
+ * @throws Exception
+ */
+ Ensembleable<T> fromConfig(long config) throws Exception;
--- End diff --
It's optional. Here's the Javadoc comment:
```
* @param fromConfig
* version of the current configuration (optional - causes reconfiguration to throw an exception if configuration is no longer current)
```
Also, in the ZK code,
```
long configId = reconfigRequest.getCurConfigId();
if (configId != -1 && configId!=lzks.self.getLastSeenQuorumVerifier().getVersion()){
```
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by Randgalt <gi...@git.apache.org>.
Github user Randgalt commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-62544636
I can't pull the new changes. I'll just pull from your personal repo I guess.
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by Randgalt <gi...@git.apache.org>.
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/curator/pull/53#discussion_r20752095
--- Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java ---
@@ -0,0 +1,29 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.curator.framework.api;
+
+public interface Configurable<T> {
+
+ /**
+ * Sets the configuration version to use.
+ * @param config The version of the configuration.
+ * @throws Exception
+ */
+ Ensembleable<T> fromConfig(long config) throws Exception;
--- End diff --
right now, fromConfig() is required. It should be optional. i.e. I should be able to write:
```
client.reconfig().leaving("").joining("").forEnsemble();
```
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by iocanel <gi...@git.apache.org>.
Github user iocanel commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-62544542
Yes, I did push to my personal repo so that the pr gets updated.
On Tue, Nov 11, 2014 at 3:07 PM, Jordan Zimmerman <no...@github.com>
wrote:
> I think you've only pushed to your personal repo. Please push to the
> apache repo too.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/curator/pull/53#issuecomment-62544094>.
>
--
*Ioannis Canellos*
*Blog: http://iocanel.blogspot.com <http://iocanel.blogspot.com/>*
*Twitter: iocanel*
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by iocanel <gi...@git.apache.org>.
Github user iocanel commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-62510702
I squashed the commits locally and pushed. I haven't pushed yet the watcher fix on GetConfigBuilderImpl and also anything related to the DynamicEnsembleProvider.
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by iocanel <gi...@git.apache.org>.
Github user iocanel commented on a diff in the pull request:
https://github.com/apache/curator/pull/53#discussion_r20758041
--- Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java ---
@@ -0,0 +1,29 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.curator.framework.api;
+
+public interface Configurable<T> {
+
+ /**
+ * Sets the configuration version to use.
+ * @param config The version of the configuration.
+ * @throws Exception
+ */
+ Ensembleable<T> fromConfig(long config) throws Exception;
--- End diff --
Right. Let me fix that!
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by iocanel <gi...@git.apache.org>.
Github user iocanel commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-62378548
After the detailed feedback, I applied all the necessary changes and squashed.
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by iocanel <gi...@git.apache.org>.
Github user iocanel commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-62555120
I've pushed additional stuff in my repo, that also contain the
EnsembleProvider and Tracker.
I can force push to the apache/CURATOR-160 if you'd like.
On Tue, Nov 11, 2014 at 3:13 PM, Jordan Zimmerman <no...@github.com>
wrote:
> I can't pull the new changes. I'll just pull from your personal repo I
> guess.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/curator/pull/53#issuecomment-62544636>.
>
--
*Ioannis Canellos*
*Blog: http://iocanel.blogspot.com <http://iocanel.blogspot.com/>*
*Twitter: iocanel*
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by iocanel <gi...@git.apache.org>.
Github user iocanel commented on a diff in the pull request:
https://github.com/apache/curator/pull/53#discussion_r20755878
--- Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java ---
@@ -0,0 +1,29 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.curator.framework.api;
+
+public interface Configurable<T> {
+
+ /**
+ * Sets the configuration version to use.
+ * @param config The version of the configuration.
+ * @throws Exception
+ */
+ Ensembleable<T> fromConfig(long config) throws Exception;
--- End diff --
Hmmm, my understanding was that the configuration version was required by ZooKeeper itself. AFAIR, not providing a valid configuration version always resulted in error. Are you sure that its optional?
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by Randgalt <gi...@git.apache.org>.
Github user Randgalt commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-62473684
I'm not seeing the latest code. Did you push it?
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by kondratovich <gi...@git.apache.org>.
Github user kondratovich commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-90881347
```java
// org/apache/curator/framework/imps/GetConfigBuilderImpl.java#52
@Override
public Ensembleable<byte[]> storingStatIn(Stat stat) {
this.stat = new Stat();
return this;
}
````
Seems you are ignoring specified `Stat` object.
---
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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by kondratovich <gi...@git.apache.org>.
Github user kondratovich commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-90653266
Hi, any update 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 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] curator pull request: [CURATOR-160] Add builders and dsl for ZooKe...
Posted by Randgalt <gi...@git.apache.org>.
Github user Randgalt commented on the pull request:
https://github.com/apache/curator/pull/53#issuecomment-62544094
I think you've only pushed to your personal repo. Please push to the apache repo too.
---
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.
---