Skip to content

Conversation

@AlvaroVega
Copy link
Member

@AlvaroVega AlvaroVega commented Nov 17, 2020

Comes from #1860

This PR will be used to align original branch to current master

New Arcgis sink to avoid licence problems with SDK file.

[cygnus-ngsi][feature][NGSIArcgisSink] create NGSIArcgisSink #1672
[cygnus-ngsi][NameMapping] Allow deep Json attribute mapping. #1856

Co-authored-by: Fermín Galán Márquez <[email protected]>
Copy link
Member Author

@AlvaroVega AlvaroVega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

<dependencies>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird indent in the added XML fragments...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -1,4 +1,4 @@
# <a name="top"></a>NGSIArcGisSink
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has been renamed. Review references to it in other parts of the documenation (with grep it should be easy), so links can be fixed.

At least I think that .mkdocs.yml should be adjusted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 33 to 51
import static com.telefonica.iot.cygnus.sinks.Enums.DataModel.DMBYATTRIBUTE;
import static com.telefonica.iot.cygnus.sinks.Enums.DataModel.DMBYENTITY;
import static com.telefonica.iot.cygnus.sinks.Enums.DataModel.DMBYSERVICE;
import static com.telefonica.iot.cygnus.sinks.Enums.DataModel.DMBYSERVICEPATH;
import java.util.Map;
import com.telefonica.iot.cygnus.utils.CommonConstants;
import com.telefonica.iot.cygnus.utils.NGSIConstants;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import org.apache.flume.Channel;
import org.apache.flume.Context;
import org.apache.flume.Event;
import org.apache.flume.EventDeliveryException;
import org.apache.flume.Sink.Status;
import org.apache.flume.Transaction;
import org.apache.flume.ChannelException;
import org.apache.flume.conf.Configurable;
import org.apache.log4j.MDC;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand why this lines were removed. I thing the code will not compile without this lines.

They should be aggregated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake. I didn't look closely, the lines were moved from its place.

NTC

Comment on lines 144 to 152
<!-- SKIP TEST -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.12.4</version>
<configuration>
<skipTests>true</skipTests>
</configuration>
</plugin>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really needed? Which new code is using it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used by com.telefonica.iot.cygnus.backends.arcgis.CredentialRestApiTest

As you can see in https://travis-ci.org/github/telefonicaid/fiware-cygnus/jobs/744408999 without this plugin we obtain:

Results :

Failed tests:   getObjectIdTest(com.telefonica.iot.cygnus.backends.arcgis.FeatureTest): Encuentra id cuando no debería haberlo.

  checkResponseTest(com.telefonica.iot.cygnus.backends.arcgis.RestApiTest): Response should be ok.

Tests in error: 

  testUserCredential(com.telefonica.iot.cygnus.backends.arcgis.CredentialRestApiTest): Can´t parse token generation url, MalformedURLException.

  getSetObjectId(com.telefonica.iot.cygnus.backends.arcgis.FeatureTest): java.lang.Integer cannot be cast to java.lang.Long

Tests run: 220, Failures: 2, Errors: 2, Skipped: 4

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.12.4:test (default-test) on project cygnus-common: There are test failures.

[ERROR] 

[ERROR] Please refer to /home/travis/build/telefonicaid/fiware-cygnus/cygnus-common/target/surefire-reports for the individual test results.

[ERROR] -> [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

<<<<<<<<<<<< Run cygnus-ngsi tests >>>>>>>>>>>>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. NTC

The intent stills looking a bit ugly. I have tried to fixed, without success. It seems to be some kind of tabs vs space issue, but it doesn't worth the time to keep trying.

* @param username
* @param password
* @param getTokenUrl
* @param b
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param b
* @param timeoutSecs

Weird... maybe there are another cases of "not fully replaced JavaDoc templates" like this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6cc4a5a

Copy link
Contributor

@IvanHdzC IvanHdzC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AlvaroVega
Copy link
Member Author

LGTM

1 similar comment
@IvanHdzC
Copy link
Contributor

LGTM

@fgalan fgalan merged commit 3a46935 into master Nov 19, 2020
@fgalan fgalan deleted the task/argis_reactor_stdr branch November 19, 2020 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants