Copilot commented on code in PR #759:
URL: https://github.com/apache/unomi/pull/759#discussion_r3261502842


##########
itests/pom.xml:
##########
@@ -247,43 +259,64 @@
                         </executions>
                     </plugin>
                     <plugin>
-                        <groupId>com.github.alexcojocaru</groupId>
-                        <artifactId>elasticsearch-maven-plugin</artifactId>
-                        <!-- REPLACE THE FOLLOWING WITH THE PLUGIN VERSION YOU 
NEED -->
-                        <version>6.29</version>
+                        <groupId>io.fabric8</groupId>
+                        <artifactId>docker-maven-plugin</artifactId>
                         <configuration>
-                            <!-- 
<downloadUrl>https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-9.2.1-darwin-aarch64.tar
-                            .gz</downloadUrl> -->
-                            
<clusterName>contextElasticSearchITests</clusterName>
-                            <transportPort>9500</transportPort>
-                            <httpPort>9400</httpPort>
-                            <version>${elasticsearch.test.version}</version>
-                            <autoCreateIndex>true</autoCreateIndex>
-                            
<instanceStartupTimeout>120</instanceStartupTimeout>
-                            <environmentVariables>
-                                <ES_JAVA_OPTS>-Xms4g -Xmx4g</ES_JAVA_OPTS>
-                            </environmentVariables>
-                            <instanceSettings>
-                                <properties>
-                                    <xpack.ml.enabled>false</xpack.ml.enabled>
-                                    
<path.repo>${project.build.directory}/snapshots_repository</path.repo>
-                                    
<cluster.routing.allocation.disk.threshold_enabled>false</cluster.routing.allocation.disk.threshold_enabled>
-                                    
<http.cors.allow-methods>OPTIONS,HEAD,GET,POST,PUT,DELETE</http.cors.allow-methods>
-                                    
<http.cors.allow-headers>Authorization,X-Requested-With,X-Auth-Token,Content-Type,Content-Length</http.cors.allow-headers>
-                                </properties>
-                            </instanceSettings>
+                            
<containerNamePattern>itests-elasticsearch</containerNamePattern>
+                            <images>
+                                <image>
+                                    
<name>docker.elastic.co/elasticsearch/elasticsearch:${elasticsearch.test.version}</name>
+                                    <alias>elasticsearch</alias>
+                                    <run>
+                                        <ports>
+                                            
<port>${elasticsearch.port}:9200</port>
+                                        </ports>
+                                        <env>
+                                            
<discovery.type>single-node</discovery.type>
+                                            <ES_JAVA_OPTS>-Xms8g -Xmx8g 
-Dcluster.default.index.settings.number_of_replicas=0</ES_JAVA_OPTS>

Review Comment:
   The Elasticsearch container heap is hardcoded to `-Xms8g -Xmx8g`, while the 
OpenSearch profile uses 4g. 8g can exceed RAM limits on common CI runners and 
developer machines and cause the container to fail to start or the build to 
OOM. Consider making the heap size configurable via a Maven property (with a 
safer default, e.g., 4g) and/or keeping Elasticsearch and OpenSearch aligned.
   



##########
itests/pom.xml:
##########
@@ -247,43 +259,64 @@
                         </executions>
                     </plugin>
                     <plugin>
-                        <groupId>com.github.alexcojocaru</groupId>
-                        <artifactId>elasticsearch-maven-plugin</artifactId>
-                        <!-- REPLACE THE FOLLOWING WITH THE PLUGIN VERSION YOU 
NEED -->
-                        <version>6.29</version>
+                        <groupId>io.fabric8</groupId>
+                        <artifactId>docker-maven-plugin</artifactId>
                         <configuration>
-                            <!-- 
<downloadUrl>https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-9.2.1-darwin-aarch64.tar
-                            .gz</downloadUrl> -->
-                            
<clusterName>contextElasticSearchITests</clusterName>
-                            <transportPort>9500</transportPort>
-                            <httpPort>9400</httpPort>
-                            <version>${elasticsearch.test.version}</version>
-                            <autoCreateIndex>true</autoCreateIndex>
-                            
<instanceStartupTimeout>120</instanceStartupTimeout>
-                            <environmentVariables>
-                                <ES_JAVA_OPTS>-Xms4g -Xmx4g</ES_JAVA_OPTS>
-                            </environmentVariables>
-                            <instanceSettings>
-                                <properties>
-                                    <xpack.ml.enabled>false</xpack.ml.enabled>
-                                    
<path.repo>${project.build.directory}/snapshots_repository</path.repo>
-                                    
<cluster.routing.allocation.disk.threshold_enabled>false</cluster.routing.allocation.disk.threshold_enabled>
-                                    
<http.cors.allow-methods>OPTIONS,HEAD,GET,POST,PUT,DELETE</http.cors.allow-methods>
-                                    
<http.cors.allow-headers>Authorization,X-Requested-With,X-Auth-Token,Content-Type,Content-Length</http.cors.allow-headers>
-                                </properties>
-                            </instanceSettings>
+                            
<containerNamePattern>itests-elasticsearch</containerNamePattern>
+                            <images>
+                                <image>
+                                    
<name>docker.elastic.co/elasticsearch/elasticsearch:${elasticsearch.test.version}</name>
+                                    <alias>elasticsearch</alias>
+                                    <run>
+                                        <ports>
+                                            
<port>${elasticsearch.port}:9200</port>
+                                        </ports>
+                                        <env>
+                                            
<discovery.type>single-node</discovery.type>
+                                            <ES_JAVA_OPTS>-Xms8g -Xmx8g 
-Dcluster.default.index.settings.number_of_replicas=0</ES_JAVA_OPTS>

Review Comment:
   `ES_JAVA_OPTS` is currently setting 
`-Dcluster.default.index.settings.number_of_replicas=0`. `-D...` only defines a 
JVM system property and won’t be applied as an Elasticsearch setting, so the 
cluster default replicas likely remain unchanged. Consider moving 
`cluster.default.index.settings.number_of_replicas=0` into its own `<env>` 
entry (like the other ES settings) or passing it as an Elasticsearch `-E` 
setting instead of a JVM `-D` property.
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to