Implementing possibility for reprojection Coordinates #1044

flomickl · 2023-01-05T22:01:48Z

Purpose

Implement #797

Remarks

I created some follow-up tickets like #1041 #1042 and #1043 to split this big change into separate tasks.
Last time it took so long that the issue was too old to be merged.

PR introduces (a) deprecation(s): no
Implemented the Apache SIS maven package.

bossenti

Hi @flomickl
just had a quick look on your PR. Could you please specify/describe to which issue this PR relates to?
It doesn't have to belong to one but I think the issue you mentioned in the title is not realy related to the changes in your PR or am I wrong?
More than that, I am bit curious why you have this empty SQL scripts in the PR. Will they be filled in an upcoming PR?
Lastly, you indicated in the PR description that this PR indtroduces deprecations but I couldn't find any @deprecation annotation somewhere. Could you please add them together with a short discpription in the PR/issue?
That would be awesome.

Thanks a lot already

flomickl · 2023-01-06T20:05:51Z

@bossenti
Hi, sorry I added the wrong ticket yesterday evening. I updated it now. It is the coordinate reprojection processor. A while ago, I created already a PR but because of the many pushes in the dev branch, it was too complicated to merge.
So I created a new, clean version.

The empty SQL Scripts are correct. This is for a clean startup in the corresponding docker-compose file.
So it is guaranteed that the container is built, even without the input data.

This way the processor does not run in any time out, which takes longer anyway from the user perspective. Instead, there is a check in the onInvocation method if the database is set up with data. If not, an SpRuntime Exception is thrown.
How to add the data is written in the README.

There are some follow-up tickets related to this issue:
#1041 #1042 #1043

Mainly the processor does some coordinate reprojection so transform some WGS84 coordinates (lat long) to an UTM projection.
Hope this makes is clearer now. I will add some longer descriptions in the wiki, to prepare the documentation.

flomickl · 2023-01-06T20:07:51Z

i don't understand the validate-pr / run-rat-plugin check and why it failed?
What does this mean?

Error:  Failed to execute goal org.apache.rat:apache-rat-plugin:0.13:check (license-check) on project streampipes-parent: Too many files with unapproved license: 4 See RAT report in: /home/runner/work/streampipes/streampipes/target/rat.txt -> [Help 1]
Error:  
Error:  To see the full stack trace of the errors, re-run Maven with the -e switch.
Error:  Re-run Maven using the -X switch to enable full debug logging.
Error:  
Error:  For more information about the errors and possible solutions, please read the following articles:
Error:  [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
Error: Process completed with exit code 1.

bossenti · 2023-01-06T21:23:02Z

@flomickl thanks a lot for your explanation 🙏🏼

With regard to the failing build: Maven complains here that it found four files without an Apache header.
You can find an overview here:
https://github.com/apache/streampipes/actions/runs/3850696943/jobs/6561136292#step:4:14597

tldr; these are the four SQL files

flomickl · 2023-01-07T07:18:59Z

@bossenti
Ah ok.

What to do with this kind of files?
Exclude them? They have anyway another header format due commend syntax --.
So like markdown or java, there is a third option now.

In
PostgreSQL_Data_Script.sql
PostgreSQL_FKey_Script.sql
PostgreSQL_Table_Script.sql
following information could be placed:

-- Usage the data is in accordance to your user profile terms of use.  The 
-- terms of use can be found here: http://www.epsg-registry.org/help/xml/Terms_Of_Use.html

EPSG_FINISH.sql is Apache conform and therefore the following could be added:

--
  -- 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.
  --
--

What do you think?

bossenti · 2023-01-07T07:25:03Z

The suggested header looks fine to me
@tenthe do we need to add the new header type somewhere in Maven?

In
PostgreSQL_Data_Script.sql
PostgreSQL_FKey_Script.sql
PostgreSQL_Table_Script.sql
following information could be placed:
-- Usage the data is in accordance to your user profile terms of use. The
-- terms of use can be found here: http://www.epsg-registry.org/help/xml/Terms_Of_Use.html

@flomickl I don't really get why we cannot add our header here? 🙈 These are just empty files, right?
But if anything prevents us here from adding an Apache header, I would exclude them.

flomickl · 2023-01-12T07:06:04Z

@bossenti Hm if you mention it. Nothing prevent it to include the license text to an empty script that reference to another file.
I will add them

flomickl · 2023-01-12T16:36:54Z

@bossenti added and style check complete. Anything else to do before merge?

bossenti · 2023-01-12T18:23:39Z

@flomickl that's great

It would be great if you could rebase your branch once but then I think you are ready to go

flomickl · 2023-01-13T06:14:29Z

rebase done :)

bossenti · 2023-01-13T07:04:33Z

@flomickl there seems to be something wrong...
Your diff is huge

flomickl · 2023-01-13T20:09:14Z

oh man, why?
I just did a simple rebase and this was the result

what to do now? @bossenti

bossenti · 2023-01-14T07:11:17Z

@flomickl it's hard to tell from the sidelines, so I took a quick look locally
I suspect you rebased the branch with an outdated version of dev, so I just did another rebase with the current version of dev
As far as I can see the diff now looks as expected, can you please check this briefly?

tenthe · 2023-01-14T07:37:02Z

streampipes-extensions/streampipes-processors-geo-jvm/pom.xml

@@ -63,7 +63,17 @@
        <dependency>
            <groupId>org.locationtech.jts</groupId>
            <artifactId>jts-core</artifactId>
-            <version>1.16.1</version>
+            <version>1.19.0</version>


Hi @flomickl can you please add the dependencies to the tag in the parent pom.
Then you don't need the version number in this pom.
In the parent pom you can also create a variable like it is done for other dependencies.
If you have any questions, just let me know.

@tenthe You mean here?
https://github.com/apache/streampipes/blob/dev/pom.xml ?

What is the difference between these two pom files?
The change should look something like:

... <jts.version>1.19.0</jts.version> ... <dependency> <groupId>org.locationtech.jts<</groupId> <artifactId>jts-core</artifactId> <version>${jts.version}</version> </dependency>

Should I do it for these entries?

jts

postgresql -> pretty sure that this entry already exist sin the parent pom

sis

com.squareup.okhttp

I did not add

<dependency> <groupId>com.squareup.okhttp3</groupId> <artifactId>okhttp</artifactId> <version>3.13.1</version> </dependency>

The entry in the geo.jvm pom should also be changed to following entry?

<dependency> <groupId>org.locationtech.jts<</groupId> <artifactId>jts-core</artifactId> <version>${jts.version}</version> </dependency>

Hey @flomickl,
the parent pom ensures that we use the same version of a dependency within the whole project.
Therefore, the dependency must be added to the <dependencyManagement> tag with the version in the parent pom.xml.
To use this dependency you can add it the to the pom.xml in the module that requires the dependency to the <dependencies> tag. The version should only be defined in the parent pom.xml and not in the module pom.xml.

The only exception are the StreamPipes dependencies.

Does this help?

@tenthe I updated the pom files.
I also run manual tests again. Everything as expected.
I left this inside, but sure that this could be removed in another ticket. The version is also outdated in both pom files.

<dependency> <groupId>com.squareup.okhttp3</groupId> <artifactId>okhttp</artifactId> <version>3.13.1</version> </dependency>

The parent pom would also need new code format process in an extra commit!
Did not want to add this here as well

bossenti · 2023-01-17T16:13:44Z

@flomickl I just gave it another rebase to keep up with recent changes 🙂

flomickl · 2023-02-01T16:20:59Z

@bossenti btw I found out what happened. Somehow, the rebase command was setup with the --onto flag in my git configs

flomickl · 2023-02-01T23:44:46Z

@dominikriemer can you check the code as well please? O:-)

flomickl · 2023-02-03T15:32:35Z

@bossenti there was a conflict in the pom file, so I fixed it here on the web IDE. Is it correct that the dev branch should be merged here? Is this a default behavior of GitHub and the web IDE or should it be a rebased? I didn't want to make it as complicated as last time....

bossenti · 2023-02-06T19:21:27Z

@flomickl I don't think we have a preference for rebasing or merge commits in this project
So creating a merge commit is just fine

flomickl requested a review from dominikriemer January 5, 2023 22:01

bossenti reviewed Jan 6, 2023

View reviewed changes

bossenti changed the title ~~Sp 584~~ Implementing possibility for reprojection Coordinates Jan 6, 2023

bossenti linked an issue Jan 6, 2023 that may be closed by this pull request

Implementing possibility for reprojection Coordinates #797

Closed

bossenti force-pushed the SP-584 branch from 0fb1a6e to 0641223 Compare January 14, 2023 07:08

tenthe requested changes Jan 14, 2023

View reviewed changes

flomickl added 8 commits January 17, 2023 17:13

[STREAMPIPES-584] Add compose files and dummy scripts

90c5c46

[STREAMPIPES-584] add processor class and additional work

ca1b6fe

[STREAMPIPES-584] Add basic documentation

b9c234c

[STREAMPIPES-584] add pop new ressources

7213e57

[STREAMPIPES-584] add connection and processor

8129f2d

[STREAMPIPES-584] Extend Connections Parameters

ae957ff

[STREAMPIPES-584] Add Readme

0434854

[STREAMPIPES-584] Add License headers

667a3e4

bossenti force-pushed the SP-584 branch from 0641223 to 667a3e4 Compare January 17, 2023 16:13

bossenti added a commit to Harry262530/streampipes that referenced this pull request Jan 25, 2023

[apache#1044] change job name

92e7028

[STREAMPIPES-584] pom files adjustments

6b65997

flomickl requested a review from tenthe February 2, 2023 23:04

tenthe approved these changes Feb 3, 2023

View reviewed changes

Merge branch 'dev' into SP-584

813fc71

flomickl mentioned this pull request Feb 3, 2023

Problem with new CouchDB and InfluxDB setup in CLI? #1221

Closed

tenthe merged commit 111e155 into dev Feb 6, 2023

bossenti deleted the SP-584 branch March 11, 2023 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing possibility for reprojection Coordinates #1044

Implementing possibility for reprojection Coordinates #1044

flomickl commented Jan 5, 2023 •

edited by bossenti

Loading

bossenti left a comment

flomickl commented Jan 6, 2023

flomickl commented Jan 6, 2023

bossenti commented Jan 6, 2023 •

edited

Loading

flomickl commented Jan 7, 2023 •

edited

Loading

bossenti commented Jan 7, 2023 •

edited

Loading

flomickl commented Jan 12, 2023

flomickl commented Jan 12, 2023

bossenti commented Jan 12, 2023

flomickl commented Jan 13, 2023

bossenti commented Jan 13, 2023

flomickl commented Jan 13, 2023

bossenti commented Jan 14, 2023

tenthe Jan 14, 2023

flomickl Feb 1, 2023

tenthe Feb 2, 2023

flomickl Feb 2, 2023

flomickl Feb 2, 2023

bossenti commented Jan 17, 2023

flomickl commented Feb 1, 2023 •

edited

Loading

flomickl commented Feb 1, 2023

flomickl commented Feb 3, 2023

bossenti commented Feb 6, 2023

Implementing possibility for reprojection Coordinates #1044

Implementing possibility for reprojection Coordinates #1044

Conversation

flomickl commented Jan 5, 2023 • edited by bossenti Loading

Purpose

Remarks

bossenti left a comment

Choose a reason for hiding this comment

flomickl commented Jan 6, 2023

flomickl commented Jan 6, 2023

bossenti commented Jan 6, 2023 • edited Loading

flomickl commented Jan 7, 2023 • edited Loading

bossenti commented Jan 7, 2023 • edited Loading

flomickl commented Jan 12, 2023

flomickl commented Jan 12, 2023

bossenti commented Jan 12, 2023

flomickl commented Jan 13, 2023

bossenti commented Jan 13, 2023

flomickl commented Jan 13, 2023

bossenti commented Jan 14, 2023

tenthe Jan 14, 2023

Choose a reason for hiding this comment

flomickl Feb 1, 2023

Choose a reason for hiding this comment

tenthe Feb 2, 2023

Choose a reason for hiding this comment

flomickl Feb 2, 2023

Choose a reason for hiding this comment

flomickl Feb 2, 2023

Choose a reason for hiding this comment

bossenti commented Jan 17, 2023

flomickl commented Feb 1, 2023 • edited Loading

flomickl commented Feb 1, 2023

flomickl commented Feb 3, 2023

bossenti commented Feb 6, 2023

flomickl commented Jan 5, 2023 •

edited by bossenti

Loading

bossenti commented Jan 6, 2023 •

edited

Loading

flomickl commented Jan 7, 2023 •

edited

Loading

bossenti commented Jan 7, 2023 •

edited

Loading

flomickl commented Feb 1, 2023 •

edited

Loading