You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/02/25 10:01:27 UTC

[GitHub] [maven-resolver] cstamas opened a new pull request #157: [MRESOLVER-245] Isolate and update tests

cstamas opened a new pull request #157:
URL: https://github.com/apache/maven-resolver/pull/157


   Main intent is to properly isolate ITs, but did other cleanup as well
   like patch update of Hazelcast and documentation updated as well.
   
   High level changes:
   * POM update with new patch release of HZ
   * removed obsolete `destroySemaphore` parameter (always false), factored out semaphore provision
   * updated all 4 ITs to form random named clusters (hence to prevent outer HZ instance joining)
   * Tests reformatted to maven codestyle (changes best viewed with `?w=1`)
   
   ---
   
   https://issues.apache.org/jira/browse/MRESOLVER-245


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on pull request #157: [MRESOLVER-245] Isolate and update tests

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #157:
URL: https://github.com/apache/maven-resolver/pull/157#issuecomment-1050738161


   Will check later this day


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on a change in pull request #157: [MRESOLVER-245] Isolate and update tests

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #157:
URL: https://github.com/apache/maven-resolver/pull/157#discussion_r820147133



##########
File path: maven-resolver-named-locks-hazelcast/src/site/markdown/index.md.vm
##########
@@ -19,40 +19,60 @@ specific language governing permissions and limitations
 under the License.
 -->
 
-This module implement named locks using Hazelcast. It provides two implementations, that are distributed and rely on
-Hazelcast 4.x ISemaphores.
+This module implement named locks using Hazelcast. It provides two implementations, using members or clients and
+rely on Hazelcast ISemaphore instances.
 
 Out of the box "hazelcast" (distributed) named lock implementations are the following:
 
-- `semaphore-hazelcast` implemented in `org.eclipse.aether.named.hazelcast.HazelcastCPSemaphoreNamedLockFactory` that uses
-  Hazelcast backed `com.hazelcast.cp.ISemaphore`. Full Hazelcast member is used here.
+- `semaphore-hazelcast` implemented in `org.eclipse.aether.named.hazelcast.HazelcastCPSemaphoreNamedLockFactory`.
+  Full Hazelcast member is used here.
 - `semaphore-hazelcast-client` implemented in `org.eclipse.aether.named.hazelcast.HazelcastClientCPSemaphoreNamedLockFactory`
-  that uses Hazelcast Client backed `com.hazelcast.cp.ISemaphore`. This implementation uses Hazelcast Client, so
-  connection to some existing Hazelcast cluster, and its existence is a requirement.
+  This implementation uses Hazelcast Client, so existing Hazelcast cluster to connect to is a requirement.
+
+The semaphore name prefix is `mvn:resolver:` and pre-configuring semaphores in Hazelcast is a must. The Hazelcast
+configuration must have the following entry:
+
+- Name prefix: `mvn:resolver:` (use pattern matching configuration)
+- JDK compatible: true
+- Permit count: `Integer.MAX_VALUE` (value `2147483647`)
+
+Example Hazelcast XML relevant snippet:
+
+```xml
+    <cp-subsystem>
+        <semaphores>
+            <semaphore>
+                <name>mvn:resolver:*</name>

Review comment:
       here

##########
File path: maven-resolver-named-locks-hazelcast/src/site/markdown/index.md.vm
##########
@@ -19,40 +19,60 @@ specific language governing permissions and limitations
 under the License.
 -->
 
-This module implement named locks using Hazelcast. It provides two implementations, that are distributed and rely on
-Hazelcast 4.x ISemaphores.
+This module implement named locks using Hazelcast. It provides two implementations, using members or clients and
+rely on Hazelcast ISemaphore instances.
 
 Out of the box "hazelcast" (distributed) named lock implementations are the following:
 
-- `semaphore-hazelcast` implemented in `org.eclipse.aether.named.hazelcast.HazelcastCPSemaphoreNamedLockFactory` that uses
-  Hazelcast backed `com.hazelcast.cp.ISemaphore`. Full Hazelcast member is used here.
+- `semaphore-hazelcast` implemented in `org.eclipse.aether.named.hazelcast.HazelcastCPSemaphoreNamedLockFactory`.
+  Full Hazelcast member is used here.
 - `semaphore-hazelcast-client` implemented in `org.eclipse.aether.named.hazelcast.HazelcastClientCPSemaphoreNamedLockFactory`
-  that uses Hazelcast Client backed `com.hazelcast.cp.ISemaphore`. This implementation uses Hazelcast Client, so
-  connection to some existing Hazelcast cluster, and its existence is a requirement.
+  This implementation uses Hazelcast Client, so existing Hazelcast cluster to connect to is a requirement.
+
+The semaphore name prefix is `mvn:resolver:` and pre-configuring semaphores in Hazelcast is a must. The Hazelcast

Review comment:
       Same here

##########
File path: maven-resolver-named-locks-hazelcast/src/site/markdown/index.md.vm
##########
@@ -19,40 +19,60 @@ specific language governing permissions and limitations
 under the License.
 -->
 
-This module implement named locks using Hazelcast. It provides two implementations, that are distributed and rely on
-Hazelcast 4.x ISemaphores.
+This module implement named locks using Hazelcast. It provides two implementations, using members or clients and
+rely on Hazelcast ISemaphore instances.
 
 Out of the box "hazelcast" (distributed) named lock implementations are the following:
 
-- `semaphore-hazelcast` implemented in `org.eclipse.aether.named.hazelcast.HazelcastCPSemaphoreNamedLockFactory` that uses
-  Hazelcast backed `com.hazelcast.cp.ISemaphore`. Full Hazelcast member is used here.
+- `semaphore-hazelcast` implemented in `org.eclipse.aether.named.hazelcast.HazelcastCPSemaphoreNamedLockFactory`.
+  Full Hazelcast member is used here.
 - `semaphore-hazelcast-client` implemented in `org.eclipse.aether.named.hazelcast.HazelcastClientCPSemaphoreNamedLockFactory`
-  that uses Hazelcast Client backed `com.hazelcast.cp.ISemaphore`. This implementation uses Hazelcast Client, so
-  connection to some existing Hazelcast cluster, and its existence is a requirement.
+  This implementation uses Hazelcast Client, so existing Hazelcast cluster to connect to is a requirement.
+
+The semaphore name prefix is `mvn:resolver:` and pre-configuring semaphores in Hazelcast is a must. The Hazelcast
+configuration must have the following entry:
+
+- Name prefix: `mvn:resolver:` (use pattern matching configuration)

Review comment:
       here

##########
File path: maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreProvider.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.named.hazelcast;
+
+/*
+ * 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.
+ */
+
+import com.hazelcast.core.HazelcastInstance;
+import com.hazelcast.cp.ISemaphore;
+
+/**
+ * Support class for providers of {@link ISemaphore} instances.
+ */
+public abstract class HazelcastSemaphoreProvider
+{
+    /**
+     * Name prefix recommended using for simpler configuration of Hazelcast.
+     */
+    protected static final String NAME_PREFIX = "mvn:resolver:";

Review comment:
       The prefix is `maven:...`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas merged pull request #157: [MRESOLVER-245] Isolate and update tests

Posted by GitBox <gi...@apache.org>.
cstamas merged pull request #157:
URL: https://github.com/apache/maven-resolver/pull/157


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on a change in pull request #157: [MRESOLVER-245] Isolate and update tests

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #157:
URL: https://github.com/apache/maven-resolver/pull/157#discussion_r821915174



##########
File path: maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreProvider.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.named.hazelcast;
+
+/*
+ * 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.
+ */
+
+import com.hazelcast.core.HazelcastInstance;
+import com.hazelcast.cp.ISemaphore;
+
+/**
+ * Support class for providers of {@link ISemaphore} instances.
+ */
+public abstract class HazelcastSemaphoreProvider
+{
+    /**
+     * Name prefix recommended using for simpler configuration of Hazelcast.
+     */
+    protected static final String NAME_PREFIX = "mvn:resolver:";

Review comment:
       That true, simple consistency with the rest as with the previous impl.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on a change in pull request #157: [MRESOLVER-245] Isolate and update tests

Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #157:
URL: https://github.com/apache/maven-resolver/pull/157#discussion_r821514970



##########
File path: maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreProvider.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.named.hazelcast;
+
+/*
+ * 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.
+ */
+
+import com.hazelcast.core.HazelcastInstance;
+import com.hazelcast.cp.ISemaphore;
+
+/**
+ * Support class for providers of {@link ISemaphore} instances.
+ */
+public abstract class HazelcastSemaphoreProvider
+{
+    /**
+     * Name prefix recommended using for simpler configuration of Hazelcast.
+     */
+    protected static final String NAME_PREFIX = "mvn:resolver:";

Review comment:
       Why does prefix matter? IMHO, even this above `mvn:resolver:...` is just redundant bytes per each semaphore created. Also, naming the HZ semaphore has no effect on anything else, nor is expected for people to "browse" HZ cluster for semaphores... so unsure why does an implementation detail (how a HZ backed named lock module names HZ semaphore) matters at all?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on a change in pull request #157: [MRESOLVER-245] Isolate and update tests

Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #157:
URL: https://github.com/apache/maven-resolver/pull/157#discussion_r822518934



##########
File path: maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreProvider.java
##########
@@ -0,0 +1,44 @@
+package org.eclipse.aether.named.hazelcast;
+
+/*
+ * 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.
+ */
+
+import com.hazelcast.core.HazelcastInstance;
+import com.hazelcast.cp.ISemaphore;
+
+/**
+ * Support class for providers of {@link ISemaphore} instances.
+ */
+public abstract class HazelcastSemaphoreProvider
+{
+    /**
+     * Name prefix recommended using for simpler configuration of Hazelcast.
+     */
+    protected static final String NAME_PREFIX = "mvn:resolver:";

Review comment:
       But what kind of "consistency" you refer to? To have same string used across unrelated modules? This prefix is an overhead, I think you get that, and if you'd ask me, all the prefix I'd use is "mvn" or not even that... :smile: 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org