etsy / kevin-middleware

This is an Express middleware that makes developing javascript in a monorepo easier.
MIT License
110 stars 5 forks source link

Add hasMaxActiveCompilersBeenReached to /compiler-info #19

Closed azu-b closed 2 years ago

azu-b commented 2 years ago

Description

It creates a function to get if Kevin is at the maximum of active compilers. It returns true if that's the case, false if not, and it is used in /compiler-info

Context / Why are we making this change?

This new info we send to /compiler-info will help to know if we are able to manually start a compiler.

Testing and QA Plan

  1. Pull this branch: abarraza_WEBPLAT-4423_show-are-we-at-max-active-compilers
  2. Use the local copy of Kevin middleware using yarnpkg link
  3. Run your server.
  4. See how many active compilers you have and hit /compiler-info. Check if it returns what you expect: true if you have the maximum of active compilers and false if not.

Questions and follow up

This PR accepts suggestions of better names of the function 🤣 , I'm not convinced with the name I gave to it.

azu-b commented 2 years ago

this is fine - as a possible optimization, it may behoove us to instead name this endpoint "compilers-info" or "compilers-status" and make the JSON object return a key value pair of string : bool instead of just a bool.

Hey @mpmartinez-etsy , now that you mentioned it, I don't know if you'd like this new idea: since we already have a /compilers-info endpoint, either we could add this information to that endpoint or we can keep this as a new endpoint and have a better name 😅 . Regarding the key-value pair, I like that idea, so I'll do it 👍

mpmartinez-etsy commented 2 years ago

this is fine - as a possible optimization, it may behoove us to instead name this endpoint "compilers-info" or "compilers-status" and make the JSON object return a key value pair of string : bool instead of just a bool.

Hey @mpmartinez-etsy , now that you mentioned it, I don't know if you'd like this new idea: since we already have a /compilers-info endpoint, either we could add this information to that endpoint or we can keep this as a new endpoint and have a better name 😅 . Regarding the key-value pair, I like that idea, so I'll do it 👍

Ooooh, I do like that idea. Does it return data in json format? if it does, I'd vote that this goes into that endpoint, then no need to create anything at all

azu-b commented 2 years ago

this is fine - as a possible optimization, it may behoove us to instead name this endpoint "compilers-info" or "compilers-status" and make the JSON object return a key value pair of string : bool instead of just a bool.

Hey @mpmartinez-etsy , now that you mentioned it, I don't know if you'd like this new idea: since we already have a /compilers-info endpoint, either we could add this information to that endpoint or we can keep this as a new endpoint and have a better name 😅 . Regarding the key-value pair, I like that idea, so I'll do it 👍

Ooooh, I do like that idea. Does it return data in json format? if it does, I'd vote that this goes into that endpoint, then no need to create anything at all

Yes, it returns JSON, so we'll go for it. I'll add it to /compiler-info image