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

[3.x] HTTP API healthcheck #413

Merged
merged 13 commits into from
Mar 4, 2021
Merged

[3.x] HTTP API healthcheck #413

merged 13 commits into from
Mar 4, 2021

Conversation

rennokki
Copy link
Collaborator

@rennokki rennokki commented Mar 4, 2021

The concept of having an API for the bot can ease multiple tasks:

  • sending commands to the bot via HTTP (opens up new ways of implementing SDKs for tf2autobot, implementable in any language since it's REST)
  • reading information about the BOT, including current item configurations for trading
  • periodic healthchecks performed on the bot via HTTP endpoint (useful for Docker/Kubernetes, but not limited to)

Roadmap

  • Base implementation, /health endpoint for checking readiness/aliveness
  • Uptime endpoint for reading the current uptime status
  • Configured items/bots endpoint for reading the configured items and bots
  • Tests, lots of tests

The GUI v3 rework already has an entrypoint for the API, so sticking just with healthchecks and uptime should be fine. 👍🏼

@idinium96
Copy link
Member

Love the idea. I will let the others share their thoughts too.

@arik123
Copy link
Member

arik123 commented Mar 4, 2021

I'm working on something simular - but I'm using mode-IPC for it, is gonna be used for GUI

@rennokki
Copy link
Collaborator Author

rennokki commented Mar 4, 2021

Discussed with @arik123 earlier on Discord about IPC and the need of having an API server on a bot. The main points were:

  1. Having a bot running is not going to start the second one since the API is already running on the specified port. This isn't an issue on Docker or any other distributed system since the API is isolated from external networking, and will only be accessible at their respective container names.
  2. The GUI is on its way and will act as a master for all bot processes. This is an excellent idea because it doesn't expose the bot on the network and the other functionalities on this PR are redundant, so I put a strike on them on the roadmap.

The only left issue is the healthcheck functionality. The healthcheck can pass/fail if I either:

  • call an URL present on the process (200 = pass, non-200 = fail)
  • ping a TCP socket (established = pass, rejected = fail)
  • execute a command (exit code zero = pass, exit code non-zero = fail)

I was looking for the TCP or the command execs for healthchecks, but I'm not sure which one to pick or how we can acheive this. The HTTP /health check is still good, but it doesn't work for simple processes, outside Docker/Kubernetes, excepting if a different port is set for each one.

@rennokki rennokki changed the title [3.x] HTTP API server [3.x] HTTP API healthcheck Mar 4, 2021
@idinium96
Copy link
Member

Thanks for the update. You may keep this pull request open so every progress/discussion can be seen here.
This may sound weird but a built-in GUI (not enabled by default) might be good, but not sure if that will use a lot of resources.

@arik123
Copy link
Member

arik123 commented Mar 4, 2021

it could be probably implemented such that, it would take near to nothing if not enabled, but then the whole part of GUI that I have planned - Managment of multiple bots would not work

@idinium96
Copy link
Member

it could be probably implemented such that, it would take near to nothing if not enabled, but then the whole part of GUI that I have planned - Managment of multiple bots would not work

Oh right true. in that case, probably best to make the GUI a separate app.

@rennokki
Copy link
Collaborator Author

rennokki commented Mar 4, 2021

Maybe the API can still exist (and be disabled by default) for non-GUI-based applications, like managing your own bots?

@idinium96
Copy link
Member

Maybe the API can still exist (and be disabled by default) for non-GUI-based applications, like managing your own bots?

Yes, sure thing.

@rennokki rennokki requested review from idinium96 and arik123 and removed request for idinium96 March 4, 2021 17:27
@rennokki rennokki marked this pull request as ready for review March 4, 2021 17:27
@rennokki rennokki marked this pull request as draft March 4, 2021 17:28
src/app.ts Outdated Show resolved Hide resolved
@rennokki rennokki marked this pull request as ready for review March 4, 2021 17:39
@rennokki rennokki requested a review from idinium96 March 4, 2021 17:39
Copy link
Member

@arik123 arik123 left a comment

Choose a reason for hiding this comment

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

What about setting default port to something random, so there is smaller chance of collision with other programs.

@@ -4,6 +4,7 @@ const { version: BOT_VERSION } = require('../package.json');
import { getPricer } from '@pricer/pricer';
import Pricer, { GetPricerFn } from './classes/Pricer';
import { loadOptions } from './classes/Options';
import HttpManager from './classes/HttpManager';
Copy link
Member

Choose a reason for hiding this comment

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

What about importing only if enabled, to reduce memory usage slightly, but this is not that important

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ES6 and TS don't allow importing with import from within classes.

Copy link
Member

Choose a reason for hiding this comment

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

I know, either use require or let it be like it is now

src/classes/HttpManager.ts Outdated Show resolved Hide resolved
@arik123
Copy link
Member

arik123 commented Mar 4, 2021

Also maybe add some note to the source, that tells everyone about GUI as main API provider.

@rennokki rennokki requested a review from arik123 March 4, 2021 17:55
@idinium96
Copy link
Member

idinium96 commented Mar 4, 2021

Now let say I want to use this and want to check my bot uptime via an API, and I am running my bot on a VPS, I need to type this on my browser URL:
http://<IP_ADDRESS>:<PORT>/uptime

right?

@rennokki
Copy link
Collaborator Author

rennokki commented Mar 4, 2021

@idinium96 Yes.

Copy link
Member

@idinium96 idinium96 left a comment

Choose a reason for hiding this comment

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

roger that 👍

merging before going to bed...

@idinium96 idinium96 merged commit 41b7558 into development Mar 4, 2021
@idinium96 idinium96 mentioned this pull request Mar 4, 2021
@rennokki rennokki deleted the feature/api branch March 4, 2021 19:06
idinium96 added a commit that referenced this pull request Mar 6, 2021
#411

## Added
- Mainly if you're using generic Unusual buy order feature:
    - ✨option to not automatically add bought Unusual item to the pricelist (usual because buying with Generic Unusual buy order - #412, [Wiki](https://github.com/TF2Autobot/tf2autobot/wiki/Configure-your-options.json-file#--automatic-add-invalid-unusual--)) - @joekiller
        - new options added:
            - `pricelist.autoAddInvalidUnusual.enable` (default is `false`)
            - `sendAlert.receivedUnusualNotInPricelist` (default if `true`)
- ✨HTTP API health check (and others in the future - #413, [Wiki](https://github.com/TF2Autobot/tf2autobot/wiki/Configuring-the-bot#api)) - @rennokki
    - new env variables:
       - `ENABLE_HTTP_API` (default is `false`)
       - `HTTP_API_PORT` (default is `3001`)
- ✨option to show proper item name in trade/offer summary (with "The", no short - #415, [Wiki](https://github.com/TF2Autobot/tf2autobot/wiki/Configure-your-options.json-file#-trade-summary-settings-)) - @idinium96
    - new options added:
        - `tradeSummary.showProperName` (default is `false`)

## Updates
- Partial price update feature (updated #337):
    - ⛔ do not partial price Mann Co. Supply Crate Key (#407) - @idinium96
    - ✅ add an option to exclude items for partial price update (#408, [Wiki](https://github.com/TF2Autobot/tf2autobot/wiki/Configure-your-options.json-file#--partial-price-update--)) - @idinium96
        - new options.json property: `pricelist.partialPriceUpdate.excludeSKU` (default is `[]`)
    - 💅 include name in partial price update summary (#409) - @idinium96
    - ⛔ do not perform usual update if grouped (#410) - @idinium96
- 🔨 refactor Bot.ts (#399) - @arik123
- 🔎🔑 keyPrices verification (2) and 🔨 small refactor on Pricelist.ts (#416) - @idinium96
- 🔕 ignore some error on fail action or to accept mobile confirmation (#419) - @idinium96 
- 🔨 refactor: use Map and Set (#420) - @idinium96
- 🎨 show amount offering/offered/taking/taken in overstocked/understocked review/trade summary (#421) - @idinium96
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.

3 participants