Complete Content Package notifications #10

bogdanguranda · 2017-11-10T12:00:04Z

No description provided.

coveralls · 2017-11-10T13:31:41Z

Coverage decreased (-20.7%) to 63.242% when pulling 8704909 on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-13T15:29:55Z

Coverage decreased (-20.9%) to 63.04% when pulling de92043 on feature/CP-notifications into 67a549e on master.

Added tests for collectionsDiffer.

coveralls · 2017-11-14T07:46:44Z

Coverage decreased (-17.2%) to 66.773% when pulling dffe4e6 on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-14T08:03:38Z

Coverage decreased (-17.2%) to 66.773% when pulling ca4637d on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-14T09:44:36Z

Coverage decreased (-17.2%) to 66.773% when pulling 6c7b67b on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-14T10:27:41Z

Coverage decreased (-17.2%) to 66.773% when pulling 623db2e on feature/CP-notifications into 67a549e on master.

MMoisa · 2017-11-14T15:03:04Z

differ/collectionsDiffer.go

+	return &defaultCollectionsDiffer{}
+}
+
+func (dcd *defaultCollectionsDiffer) Diff(incomingCollectionUuids []string, oldCollectionUuids []string) ([]string, map[string]bool) {


Why not just return only the map from this function? The keys from the isDeleted map are the uuids of the content that was added/deleted.

Added healthcheck for relations resolver.

coveralls · 2017-11-15T07:38:47Z

Coverage increased (+0.8%) to 84.699% when pulling 9c86996 on feature/CP-notifications into 67a549e on master.

MMoisa · 2017-11-15T08:35:42Z

healthcheck.go

+	contentResolverHealthURI   string
+	relationsResolverHealthURI string
+	producer                   producer.MessageProducer
+	client                     *http.Client
 }

 func newHealthService(config *healthConfig) *healthService {


The gtg healthcheck should be done in parallel. There is an example in the methode-content-placeholder-mapper for this.

MMoisa · 2017-11-15T09:36:41Z

resolver/contentResolver.go

 	"io/ioutil"
 	"net/http"
+
+	"github.com/Financial-Times/transactionid-utils-go"
 )

 type ContentResolver interface {


Test the flow where the httpClient can't make the call to the contentResolverApp.

Cannot test this, because of go not having an interface for http client.

MMoisa · 2017-11-15T09:39:07Z

relations/relationsResolver.go

+
+	resp, err := drr.httpClient.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("Error doing request to uri=[%v], transaction_id=[%v].", completeUri, tid)


This flow can be tested as well.

Cannot test this, because of go not having an interface for http client.

coveralls · 2017-11-15T12:24:06Z

Coverage increased (+0.9%) to 84.793% when pulling 760f625 on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-15T12:54:58Z

Coverage increased (+0.7%) to 84.579% when pulling 52fd461 on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-15T17:00:45Z

Coverage increased (+0.6%) to 84.507% when pulling 830c014 on feature/CP-notifications into 67a549e on master.

MMoisa · 2017-11-16T06:46:53Z

differ/collectionsDiffer.go

+}
+
+func oneWayDiff(firstCollection []string, secondCollection []string, markDeleted bool, mapToAdd map[string]bool) {
+	secondCollectionTemp := make(map[string]interface{})


Instead of map[string]interface{} this should be map[string]struct{} and secondCollectionTemp[secondColUuid] should be initialized with struct{}{}.

coveralls · 2017-11-21T12:10:47Z

Coverage increased (+0.6%) to 84.531% when pulling 9d03314 on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-21T12:46:05Z

Coverage increased (+0.6%) to 84.531% when pulling f762d97 on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-21T14:32:08Z

Coverage increased (+0.7%) to 84.675% when pulling 2e3c6b7 on feature/CP-notifications into 67a549e on master.

Added log for testing.

coveralls · 2017-11-21T15:18:32Z

Coverage increased (+0.8%) to 84.722% when pulling bcc053a on feature/CP-notifications into 67a549e on master.

MMoisa · 2017-11-22T07:17:24Z

README.md

+1. **relations-api** connectivity check
+2. **content-collection-neo4j-rw** connectivity check
+3. **document-store-api** connectivity check
+4. **kafka** connectivity check and topic existence check


The existence of the kafka topic is no longer checked.

sbuliarca · 2017-11-22T09:47:10Z

helm/content-collection-unfolder/app-configs/content-collection-unfolder_delivery.yaml

@@ -8,4 +8,6 @@ env:
  WRITER_HEALTH_URI: http://content-collection-rw-neo4j:8080/__health
  CONTENT_RESOLVER_URI: http://document-store-api:8080/content/
  CONTENT_RESOLVER_HEALTH_URI: http://document-store-api:8080/__health
+  RELATIONS_RESOLVER_URI: http://relations-api/contentcollection/{uuid}/relations
+  RELATIONS_RESOLVER_HEALTH_URI: http://relations-api/__health


You should access the relations-api on the 8080 port, like http://relations-api:8080/..

sbuliarca

Please fix the relations-api URL as its healthcheck is not green.

coveralls · 2017-11-22T10:17:21Z

Coverage decreased (-1.2%) to 82.743% when pulling d9350f3 on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-22T10:26:36Z

Coverage decreased (-1.2%) to 82.743% when pulling db5df41 on feature/CP-notifications into 67a549e on master.

coveralls · 2017-11-22T10:34:19Z

Coverage decreased (-1.4%) to 82.537% when pulling 15b4225 on feature/CP-notifications into 67a549e on master.

coveralls · 2017-12-04T09:30:12Z

Coverage decreased (-2.0%) to 81.94% when pulling 53b76bf on feature/CP-notifications into 67a549e on master.

coveralls · 2017-12-05T09:03:39Z

Coverage decreased (-1.9%) to 81.994% when pulling 8c7db3c on feature/CP-notifications into 67a549e on master.

coveralls · 2017-12-05T09:31:15Z

Coverage decreased (-1.4%) to 82.537% when pulling 05c9cd2 on feature/CP-notifications into 67a549e on master.

bogdanguranda added 2 commits November 10, 2017 13:56

First implementation. No tests.

60042cb

Fixed wrong URI for relations-api endpoint.

8704909

Added missing lead article to resolve content for.

de92043

Added missing isDeleted for the lead Article.

dffe4e6

Added tests for collectionsDiffer.

Added helm configuration for k8s

ca4637d

Added tests for contentProducer.

6c7b67b

Arranged imports.

623db2e

Added tests for relations resolver.

2b83f5a

MMoisa reviewed Nov 14, 2017

View reviewed changes

bogdanguranda added 5 commits November 14, 2017 20:07

K8s config update

5dad2ba

Added relations resolver health uri env var.

4f5e0da

Added healthcheck for relations resolver.

Added tests for main file.

5d35aaf

Added tests for unfolder. One more test to go.

9d97f45

Fixed last test.

3a98e82

bogdanguranda changed the title ~~First implementation. No tests.~~ Complete Content Package notifications Nov 15, 2017

Removed unneded vendor dependency.

9c86996

MMoisa reviewed Nov 15, 2017

View reviewed changes

Fixed added new collection members notifications (no old relations).

760f625

Changed logging without logEntry.

52fd461

Improved implementation for collections differ.

830c014

MMoisa reviewed Nov 16, 2017

View reviewed changes

More improvement implementation for collections differ.

9d03314

Updated readme

f762d97

Added extra logs for testing purposes.

2e3c6b7

Removed unnecessary http client from uuid and date resolver.

bcc053a

Added log for testing.

MMoisa reviewed Nov 22, 2017

View reviewed changes

bogdanguranda added 2 commits November 22, 2017 11:17

GTG parallel

d56154b

Fixed readme

d9350f3

sbuliarca reviewed Nov 22, 2017

View reviewed changes

sbuliarca suggested changes Nov 22, 2017

View reviewed changes

Fixed helm configuration, missing port for relations api

db5df41

Removed testing logs.

15b4225

MMoisa approved these changes Nov 22, 2017

View reviewed changes

sbuliarca approved these changes Nov 22, 2017

View reviewed changes

Added missing model fields.

05c9cd2

bogdanguranda force-pushed the feature/CP-notifications branch from 8c7db3c to 05c9cd2 Compare December 5, 2017 09:28

bogdanguranda merged commit f2fd762 into master Dec 6, 2017

bogdanguranda deleted the feature/CP-notifications branch December 6, 2017 10:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Complete Content Package notifications #10

Complete Content Package notifications #10

bogdanguranda commented Nov 10, 2017

coveralls commented Nov 10, 2017

coveralls commented Nov 13, 2017

coveralls commented Nov 14, 2017

coveralls commented Nov 14, 2017

coveralls commented Nov 14, 2017

coveralls commented Nov 14, 2017

MMoisa Nov 14, 2017 •

edited

coveralls commented Nov 15, 2017

MMoisa Nov 15, 2017

bogdanguranda Nov 24, 2017

MMoisa Nov 15, 2017

bogdanguranda Nov 22, 2017

MMoisa Nov 15, 2017

bogdanguranda Nov 22, 2017

coveralls commented Nov 15, 2017

coveralls commented Nov 15, 2017

coveralls commented Nov 15, 2017

MMoisa Nov 16, 2017

coveralls commented Nov 21, 2017

coveralls commented Nov 21, 2017

coveralls commented Nov 21, 2017

coveralls commented Nov 21, 2017

MMoisa Nov 22, 2017

bogdanguranda Nov 22, 2017

sbuliarca Nov 22, 2017

sbuliarca left a comment

coveralls commented Nov 22, 2017

coveralls commented Nov 22, 2017

coveralls commented Nov 22, 2017

coveralls commented Dec 4, 2017

coveralls commented Dec 5, 2017

coveralls commented Dec 5, 2017

Complete Content Package notifications #10

Complete Content Package notifications #10

Conversation

bogdanguranda commented Nov 10, 2017

coveralls commented Nov 10, 2017

coveralls commented Nov 13, 2017

coveralls commented Nov 14, 2017

coveralls commented Nov 14, 2017

coveralls commented Nov 14, 2017

coveralls commented Nov 14, 2017

MMoisa Nov 14, 2017 • edited

Choose a reason for hiding this comment

coveralls commented Nov 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 15, 2017

coveralls commented Nov 15, 2017

coveralls commented Nov 15, 2017

Choose a reason for hiding this comment

coveralls commented Nov 21, 2017

coveralls commented Nov 21, 2017

coveralls commented Nov 21, 2017

coveralls commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbuliarca left a comment

Choose a reason for hiding this comment

coveralls commented Nov 22, 2017

coveralls commented Nov 22, 2017

coveralls commented Nov 22, 2017

coveralls commented Dec 4, 2017

coveralls commented Dec 5, 2017

coveralls commented Dec 5, 2017

MMoisa Nov 14, 2017 •

edited