You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@baremaps.apache.org by "Perdjesk (via GitHub)" <gi...@apache.org> on 2023/05/16 15:05:32 UTC

[GitHub] [incubator-baremaps] Perdjesk opened a new pull request, #658: WIP - Removing --database CLI in dev/export/serve and replace with field in tileset.json

Perdjesk opened a new pull request, #658:
URL: https://github.com/apache/incubator-baremaps/pull/658

   Proposal for https://github.com/apache/incubator-baremaps/issues/655


-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196322184


##########
.run/geocoder-create.run.xml:
##########
@@ -4,8 +4,13 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.js" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/geocoding" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196321857



##########
.run/geocoder-serve.run.xml:
##########
@@ -6,6 +6,11 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="geocoder serve --index geocoder-index --port 9000" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/geocoding" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196321857



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196717356


##########
baremaps-server/src/test/java/org/apache/baremaps/server/TileSetTest.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed 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.baremaps.server;
+
+
+import static org.apache.baremaps.utils.ObjectMapperUtils.objectMapper;
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.io.File;
+import java.io.IOException;
+import org.apache.baremaps.server.TileJSON.TileJSON;
+import org.apache.baremaps.vectortile.tileset.Tileset;
+import org.junit.Test;
+
+public class TileSetTest {
+
+  String tilesetFile = "/tilesets/tileset.json";
+  String referenceFile = "/tilesets/osm.json";
+
+  private File resourceFile(String path) {
+    return new File(getClass().getResource(path).getFile());

Review Comment:
   Addressed in 1a2dabfb3a839b04b54ae20f467a9e3e88867efa



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196716717


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TileJSON/Field.java:
##########
@@ -0,0 +1,16 @@
+/*
+ * Licensed 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.baremaps.server.TileJSON;
+
+public class Field {

Review Comment:
   Addressed in 1a2dabfb3a839b04b54ae20f467a9e3e88867efa



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] bchapuis commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196398796


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TileJSON/Field.java:
##########
@@ -0,0 +1,16 @@
+/*
+ * Licensed 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.baremaps.server.TileJSON;
+
+public class Field {

Review Comment:
   Interesting, thank you for the clarification. I wonder if using an Object instead of a Field would give the flexibility to put a Map in this attribute. What do you think? Both alternatives are fine for me.
   
   ```java
   @JsonProperty("fields")
   @Valid
   @NotNull Object fields,
   ```



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196377103


##########
baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Dev.java:
##########
@@ -64,11 +60,13 @@ public class Dev implements Callable<Integer> {
 
   @Override
   public Integer call() throws Exception {
-    try (var dataSource = PostgresUtils.dataSource(database)) {
+    // Configure serialization
+    var objectMapper = objectMapper();
 
-      // Configure serialization
-      var objectMapper = objectMapper();
+    var tileSet = objectMapper.readValue(tileset.toFile(), Tileset.class);

Review Comment:
   It seems to me that this a regression with this change. All others places including `DevResources.java` are reading the file always through the `ConfigReader`. It was overlooked because, I missed that it was required due to the underlying `--tilset` file supporting to be JS or plain JSON.



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] github-code-scanning[bot] commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1195337323


##########
baremaps-server/src/test/java/org/apache/baremaps/server/TileSetTest.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed 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.baremaps.server;
+
+
+import static org.apache.baremaps.utils.ObjectMapperUtils.objectMapper;
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.io.File;
+import java.io.IOException;
+import org.apache.baremaps.server.TileJSON.TileJSON;
+import org.apache.baremaps.vectortile.tileset.Tileset;
+import org.junit.Test;
+
+public class TileSetTest {
+
+  String tilesetFile = "/tilesets/tileset.json";
+  String referenceFile = "/tilesets/osm.json";
+
+  private File resourceFile(String path) {
+    return new File(getClass().getResource(path).getFile());

Review Comment:
   ## Unsafe use of getResource
   
   The idiom getClass().getResource() is unsafe for classes that may be extended.
   
   [Show more details](https://github.com/apache/incubator-baremaps/security/code-scanning/660)



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] bchapuis merged pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis merged PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658


-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196324149


##########
.run/extrusion-create.run.xml:
##########
@@ -4,6 +4,11 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.json" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/extrusion" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">
+      <option name="credential" />
+      <option name="region" />
+      <option name="useCurrentConnection" value="false" />
+    </extension>

Review Comment:
   https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196321857



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] bchapuis commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196398796


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TileJSON/Field.java:
##########
@@ -0,0 +1,16 @@
+/*
+ * Licensed 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.baremaps.server.TileJSON;
+
+public class Field {

Review Comment:
   Interesting, thank you for the clarification. I wonder if using an Object instead of a Field would give the flexibility to put a Map in this attribute. What do you think? Both alternatives are fine for me.
   
   ```java
   @JsonProperty("fields")
   @Valid
   @NotNull Field fields,
   ```



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196321857


##########
.run/extrusion-create.run.xml:
##########
@@ -4,6 +4,11 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.json" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/extrusion" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   Addressed in https://github.com/apache/incubator-baremaps/pull/658/commits/aac687ed7e1c6bc411992610b3964ad2a750842d.
   Aligned all `run.xml` without this section as some had it previously and my IDE added it to some others.



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196367082


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TileJSON/Field.java:
##########
@@ -0,0 +1,16 @@
+/*
+ * Licensed 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.baremaps.server.TileJSON;
+
+public class Field {

Review Comment:
   It is new in tilejson-spec 3.0.0 https://github.com/mapbox/tilejson-spec/tree/master/3.0.0#332-fields 
   ```
   An object whose keys and values are the names and descriptions of attributes available in this layer. Each value (description) MUST be a string that describes the underlying data. If no fields are present, the fields key MUST be an empty object.
   ```
   
   It is empty but the class purpose would be to pick-up all the fields in a Java Map in a class attribute called `additionalProperties` (as per naming in json schema of the specification).
   
   To be discuss what as part of the points is to which extent to add support for the tilejson-spec in this change.



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196367082


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TileJSON/Field.java:
##########
@@ -0,0 +1,16 @@
+/*
+ * Licensed 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.baremaps.server.TileJSON;
+
+public class Field {

Review Comment:
   It is new in tilejson-spec 3.0.0 https://github.com/mapbox/tilejson-spec/tree/master/3.0.0#332-fields 
   
   > An object whose keys and values are the names and descriptions of attributes available in this layer. Each value (description) MUST be a string that describes the underlying data. If no fields are present, the fields key MUST be an empty object.
   
   
   It is empty but the class purpose would be to pick-up all the fields in a Java Map in a class attribute called `additionalProperties` (as per naming in json schema of the specification).
   
   To be discuss what as part of the points is to which extent to add support for the tilejson-spec in this change.



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196355431


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TileJSON/Field.java:
##########
@@ -0,0 +1,16 @@
+/*
+ * Licensed 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.baremaps.server.TileJSON;

Review Comment:
   Addressed in https://github.com/apache/incubator-baremaps/pull/658/commits/27ac21ae07f2125193b824c3200704ccce71aa7f 
   I quickly check the configuration with spotless and it is not enforced by it. Sonar has a rule for it java:S120.
   
   https://rules.sonarsource.com/java/tag/convention/RSPEC-120



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196618924


##########
baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Dev.java:
##########
@@ -64,11 +60,13 @@ public class Dev implements Callable<Integer> {
 
   @Override
   public Integer call() throws Exception {
-    try (var dataSource = PostgresUtils.dataSource(database)) {
+    // Configure serialization
+    var objectMapper = objectMapper();
 
-      // Configure serialization
-      var objectMapper = objectMapper();
+    var tileSet = objectMapper.readValue(tileset.toFile(), Tileset.class);

Review Comment:
   Addressed in 5e7b73433f3ad621947b6045a776ba71b87bf793



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196322883


##########
.run/naturalearth-create.run.xml:
##########
@@ -4,6 +4,11 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.json" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/naturalearth" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196321857



##########
.run/naturalearth-dev.run.xml:
##########
@@ -2,8 +2,13 @@
   <configuration default="false" name="naturalearth-dev" type="Application" factoryName="Application">
     <option name="MAIN_CLASS_NAME" value="org.apache.baremaps.cli.Baremaps" />
     <module name="baremaps-cli" />
-    <option name="PROGRAM_PARAMETERS" value="map dev --database jdbc:postgresql://localhost:5432/baremaps?user=baremaps&amp;password=baremaps --tileset tileset.json --style style.json" />
+    <option name="PROGRAM_PARAMETERS" value="map dev --tileset tileset.json --style style.json" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/naturalearth" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196321857



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196322448


##########
.run/iploc-create.run.xml:
##########
@@ -4,8 +4,13 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.js" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/ip-to-location" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196321857



##########
.run/iploc-serve.run.xml:
##########
@@ -4,8 +4,13 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="iploc serve --database iploc.db --port 9000" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/ip-to-location" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   https://github.com/apache/incubator-baremaps/pull/658#discussion_r1196321857



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] bchapuis commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1195332815


##########
.run/extrusion-create.run.xml:
##########
@@ -4,6 +4,11 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.json" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/extrusion" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   This section has probably been automatically added by IntelliJ. 



##########
.run/geocoder-serve.run.xml:
##########
@@ -6,6 +6,11 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="geocoder serve --index geocoder-index --port 9000" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/geocoding" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   Same here.



##########
.run/geocoder-create.run.xml:
##########
@@ -4,8 +4,13 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.js" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/geocoding" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   Same here.



##########
.run/iploc-serve.run.xml:
##########
@@ -4,8 +4,13 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="iploc serve --database iploc.db --port 9000" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/ip-to-location" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   Same here.



##########
.run/naturalearth-dev.run.xml:
##########
@@ -2,8 +2,13 @@
   <configuration default="false" name="naturalearth-dev" type="Application" factoryName="Application">
     <option name="MAIN_CLASS_NAME" value="org.apache.baremaps.cli.Baremaps" />
     <module name="baremaps-cli" />
-    <option name="PROGRAM_PARAMETERS" value="map dev --database jdbc:postgresql://localhost:5432/baremaps?user=baremaps&amp;password=baremaps --tileset tileset.json --style style.json" />
+    <option name="PROGRAM_PARAMETERS" value="map dev --tileset tileset.json --style style.json" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/naturalearth" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   Same here.



##########
baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Dev.java:
##########
@@ -64,11 +60,13 @@ public class Dev implements Callable<Integer> {
 
   @Override
   public Integer call() throws Exception {
-    try (var dataSource = PostgresUtils.dataSource(database)) {
+    // Configure serialization
+    var objectMapper = objectMapper();
 
-      // Configure serialization
-      var objectMapper = objectMapper();
+    var tileSet = objectMapper.readValue(tileset.toFile(), Tileset.class);

Review Comment:
   Not directly linked to this issue, but I realize that the dev mode does not support javascript configuration files. Here is an [example](https://github.com/apache/incubator-baremaps/blob/1bae9022ec49b192cfb7ed3db2c133c29d9d2353/baremaps-cli/src/main/java/org/apache/baremaps/cli/map/Serve.java#L73) of how the serve command works. The ConfigReader reads or executes the configuration file, then it deserializes it.



##########
.run/extrusion-dev.run.xml:
##########
@@ -2,8 +2,13 @@
   <configuration default="false" name="extrusion-dev" type="Application" factoryName="Application">
     <option name="MAIN_CLASS_NAME" value="org.apache.baremaps.cli.Baremaps" />
     <module name="baremaps-cli" />
-    <option name="PROGRAM_PARAMETERS" value="map dev --database jdbc:postgresql://localhost:5432/baremaps?user=baremaps&amp;password=baremaps --tileset tileset.json --style style.json" />
+    <option name="PROGRAM_PARAMETERS" value="map dev --tileset tileset.json --style style.json" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/extrusion" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   Same here.



##########
.run/naturalearth-create.run.xml:
##########
@@ -4,6 +4,11 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.json" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/naturalearth" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   Same here.



##########
.run/iploc-create.run.xml:
##########
@@ -4,8 +4,13 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.js" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/ip-to-location" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">

Review Comment:
   Same here.



##########
baremaps-server/src/main/java/org/apache/baremaps/server/TileJSON/Field.java:
##########
@@ -0,0 +1,16 @@
+/*
+ * Licensed 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.baremaps.server.TileJSON;
+
+public class Field {

Review Comment:
   Out of curiosity, what is a field? Does it acts as a placeholder?



##########
baremaps-server/src/main/java/org/apache/baremaps/server/TileJSON/Field.java:
##########
@@ -0,0 +1,16 @@
+/*
+ * Licensed 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.baremaps.server.TileJSON;

Review Comment:
   According to oracle the package names should be lower case.
   https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#discussion_r1195352144


##########
.run/extrusion-create.run.xml:
##########
@@ -4,6 +4,11 @@
     <module name="baremaps-cli" />
     <option name="PROGRAM_PARAMETERS" value="workflow execute --file workflow.json" />
     <option name="WORKING_DIRECTORY" value="$PROJECT_DIR$/examples/extrusion" />
+    <extension name="software.aws.toolkits.jetbrains.core.execution.JavaAwsConnectionExtension">
+      <option name="credential" />
+      <option name="region" />
+      <option name="useCurrentConnection" value="false" />
+    </extension>

Review Comment:
   TODO: some cleanup due to IDE plugins



-- 
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: commits-unsubscribe@baremaps.apache.org

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

[GitHub] [incubator-baremaps] bchapuis commented on pull request #658: Remove --database CLI in dev/export/serve and replace with field in tileset.json

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis commented on PR #658:
URL: https://github.com/apache/incubator-baremaps/pull/658#issuecomment-1557514384

   Thanks a lot for these changes. I updated the website and the documentation accordingly.


-- 
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: commits-unsubscribe@baremaps.apache.org

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