Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proof of Concept for Image Microservice #1636

Merged
merged 1 commit into from
Feb 13, 2021

Conversation

humphd
Copy link
Contributor

@humphd humphd commented Feb 2, 2021

Issue This PR Addresses

Part of #1627

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

To give us something to discuss, and a starting point for further development, I've created a simple image service.

See the README.md for info about using it.

@humphd
Copy link
Contributor Author

humphd commented Feb 2, 2021

To test this:

npm install
npm start

Now try going to http://localhost:4444/?w=1024&t=webp&u=https%3A%2F%2Fimages.unsplash.com%2Fphoto-1593642702909-dec73df255d7%3Fixid%3DMXwxMjA3fDF8MHxwaG90by1wYWdlfHx8fGVufDB8fHw%253D%26ixlib%3Drb-1.2.1%26auto%3Dformat%26fit%3Dcrop%26w%3D1350%26q%3D80 in your browser.

@humphd
Copy link
Contributor Author

humphd commented Feb 2, 2021

First problem: how to deal with all the services under src/api/* and how to install things? I could add another layer of what we already do with npm install in the root calling into a package.json in src/api/package.json and then it can call into src/api/image/package.json, etc. but there must be a cleaner way?

@humphd
Copy link
Contributor Author

humphd commented Feb 2, 2021

I notice that npm 7 is now officially released, and it includes Workspaces, which we could use to manage these apis and other package.json files.

The downside is that we force our devs to move to npm 7, which @Metropass has been working on in #1617. But it might be something we move toward in an upcoming release to get away from all these scripts to manage internal packaging.

@humphd
Copy link
Contributor Author

humphd commented Feb 3, 2021

Unrelated to this work, I came across this standard which is really cool: https://iiif.io/api/image/2.0/

@raygervais
Copy link
Contributor

raygervais commented Feb 3, 2021

First problem: how to deal with all the services under src/api/* and how to install things? I could add another layer of what we already do with npm install in the root calling into a package.json in src/api/package.json and then it can call into src/api/image/package.json, etc. but there must be a cleaner way?

I think the easiest for management is each service get's it own Docker image which can be orchestrated, I'd recommend something simple as this to start:

FROM node:alpine-lst AS base

EXPOSE 4444

WORKDIR /app

COPY . .

RUN npm install

CMD npm start

All this does is simply containerize, but with it now we can start the discussion of allowing docker-compose orchestration duties on a per service level. In my opinion, unless you are directly developing the service, having a container which contains the node_modules and service itself seems simplest. Thoughts?

src/api/image/src/app.js Outdated Show resolved Hide resolved
@humphd
Copy link
Contributor Author

humphd commented Feb 3, 2021

RE: your Dockerfile, sounds perfect. Let's start small and improve it as we go. Once we have a template, we can use it to build more.

@humphd
Copy link
Contributor Author

humphd commented Feb 3, 2021

@raygervais should I let you do PRs against my branch at this point, or do you want me to do more first? I'm assuming you'll run with what I have. Let me know if that's wrong.

@raygervais
Copy link
Contributor

I can continue here if that's alright, will add to your commits:

  • Dockerfile
  • The basic Traefik configuration under /src/api

If you want to avoid email spam, we could land this item and I work on it from there in a follow up.

@humphd
Copy link
Contributor Author

humphd commented Feb 3, 2021

I don't want to land this because it will break CI atm. Do PR(s) against my fork for now.

@raygervais
Copy link
Contributor

Next Steps:

  • Let's get CI looking green so we can merge?

@humphd
Copy link
Contributor Author

humphd commented Feb 5, 2021

One way to do this is to use workspaces in npm 7, but that would require all our devs to update to latest npm. I think it might be a good solution overall, but we could easily get this passing CI with some scripts to do the dependent installs, like we do now with other sub-package.jsons.

@humphd humphd requested review from raygervais and huyxgit February 8, 2021 00:07
@humphd
Copy link
Contributor Author

humphd commented Feb 8, 2021

This is ready for review. It's good enough to land and we can iterate on it.

@humphd
Copy link
Contributor Author

humphd commented Feb 8, 2021

Added some Docker updates to get this working properly with docker-compose, now able to run the image service + traefik!

Screen Shot 2021-02-08 at 11 57 27 AM

@HyperTHD
Copy link
Contributor

Any updates with this?

@humphd
Copy link
Contributor Author

humphd commented Feb 10, 2021

I'm pretty close to having this ready. I have two efforts happening in parallel:

  • Creating a reusable microservice base package, happening in https://github.com/Seneca-CDOT/satellite. I hope to get an initial version of that published to npm on Friday.
  • Creating the Docker, Docker-Compose, ELK (Elastic/Logstach/Kibana) logging, metrics, and monitoring stack. I did a demo of this yesterday, and it should be usable in some form by the end of this week or early next week.

I'm going to rewrite this code based on Satellite once I publish that package.

@raygervais
Copy link
Contributor

@humphd, tag me when you want to discuss next steps and also get me to review :).

@humphd
Copy link
Contributor Author

humphd commented Feb 12, 2021

@raygervais will do. I'm just fighting with APM route naming in Kibana, then I should be ready.

@humphd
Copy link
Contributor Author

humphd commented Feb 13, 2021

OK, I think this is ready. I'd like to get this landed so @chrispinkney has a template to use for his work on the User service.

I think we can improve the Docker setup, but this shouldn't block things.

@raygervais, @manekenpix when you have a sec, r?

manekenpix
manekenpix previously approved these changes Feb 13, 2021
Copy link
Member

@manekenpix manekenpix left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I could access:

  • image.docker.localhost/{image, gallery}
  • elastic.docker.localhost
  • kibana.docker.localhost (could see stats in the observability menu)

@vercel
Copy link

vercel bot commented Feb 13, 2021

Deployment failed with the following error:

Resource is limited - try again after in 7 minutes (more than 100, code: "api-deployments-free-per-day").

@vercel
Copy link

vercel bot commented Feb 13, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/humphd/telescope/545r62u3g
✅ Preview: https://telescope-git-fork-humphd-image-service.humphd.now.sh

@humphd humphd merged commit b7f4803 into Seneca-CDOT:master Feb 13, 2021
@humphd humphd added this to the 1.7 Release milestone Feb 13, 2021
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