Add service for sticker popularity tracking and Flickr searches #35
Conversation
stickers/routes/test.js
Outdated
|
|
||
| const express = require('express'); | ||
| const bodyParser = require('body-parser'); | ||
| const kafka = require('kafka-node'); |
There was a problem hiding this comment.
Mentioned this in the other PR, but probably could to clarify why we're using Kafka in a comment.
.vscode/launch.json
Outdated
| "name": "Attach (sticker service)", | ||
| "address": "localhost", | ||
| "port": 5959, | ||
| "localRoot": "${workspaceRoot}/stickers/", |
There was a problem hiding this comment.
Consider calling this dir "stickerService". Again this is for consistency - for example, I called mine "checkoutService".
There was a problem hiding this comment.
In addition to the comments I added, there are two general thoughts I want to add:
1.) I've been thinking more about how the microservices are structured, and the point that bothers me is that we have a dependency between the Session and Sticker services because they both share a common data source -e.g. the stickers stored in mongo. To follow good architecture, there are two options we could consider:
a.) Consolidate the two services since their functionality is related enough that they share common data
b.) Create yet another microservice that acts only as the interface to interacting with the data.
In many ways, Sticker Service doesn't provide much more functionality than what is mentioned for (b) above, so the current structure is probably fine. But, I wanted to bring this up to see what others thoughts are. This is probably something we'll want to explain in the sample when we provide an overview of the architecture.
2.) We had discussed this somewhere else, but it feels like there should still be another work item to hook this service up with the API Gateway and then to hook the client up with the API Gateway.
|
|
||
| services: | ||
| mongo: | ||
| image: mongo:3.5.5 |
There was a problem hiding this comment.
Why the need for a specific mongo image here? in the other compose files, we haven't done this.
Also, I made a similar comment in your other PR, but should we have separate compose files for each service or a single one other the root of the app?
Where is the prod version of the file (i'm assuming .dev implies debug)? Note that the root compose file is named docker-compose.debug.yml, so we should maintain consistency.
There was a problem hiding this comment.
There's no need per se for a particular mongo image, but specifying one is a good practice because otherwise you get :latest and forego the consistency benefit of using containers.
stickerService/debug.env
Outdated
| @@ -0,0 +1,10 @@ | |||
| DB_NAME=getStickersDemo | |||
There was a problem hiding this comment.
Similar comment in other PR - how do these values get set if you want to run the service locally, without using a container? The reason I mention this is that to make debugging easier, I think we'll want/need that ability.
stickerService/index.js
Outdated
| .catch(error => console.log(`sending top items failed: ${error}`)); | ||
| }); | ||
|
|
||
| server.listen(process.env.PORT, () => { |
There was a problem hiding this comment.
We should establish consistency across the services with how we're setting\defaulting configurable values, such as the port, so that it makes the sample code easy to follow.
The other services I've implemented use a config file to set them. Here, environment variables are used. I'm honestly fine with either approach and perhaps this is something that we come back and cleanup at the end once we have all the pieces together.
There was a problem hiding this comment.
I chose environment variables initially because they're easy and will stay that way when we deploy to Kubernetes. Config files baked into images are inconvenient there.
stickerService/index.js
Outdated
| const db = require('./mongodb'); | ||
| const test = require('./routes/test'); | ||
|
|
||
| // TODO use the gulp task for this (in an init container perhaps) |
There was a problem hiding this comment.
I made a comments else where about how the configuration settings get defaulted in the scenario where you want to run the service outside of a container (to make debugging easier) - I see this TODO comment, so it sounds like you will have a Gulp task that sets all of these? We'll want to make sure our approach for this is consistent across all the services. Probably worth a discussion with Jonathan on which way we prefer.
There was a problem hiding this comment.
The Gulp task already exists in the root gulpfile (which this service doesn't have). The TODO is more a note to myself that the task exists, and that running it in an init container would be neater than this block of code.
Re debugging, my comment on the other PR explains why I think debugging this service is already easy.
|
@nicolehaugen79 are there any other changes you'd like to see here? |
This service owns the mongodb sticker data. Other services can retrieve sticker records from its
/stickersroute, optionally filtering by ids and tags, and persist new stickers as well. Its/searchroute provides search results from the Flickr API.Additionally, this service tracks sticker popularity trends. Here's the gist:
Closes #14, #17, and #43.