Skip to content

Conversation

@eldadfux
Copy link
Member

@eldadfux eldadfux commented Jan 23, 2022

What does this PR do?

@PineappleIOnic I added a few minor fixes on top of the PR review.

  • Reorganized namespaces
  • Reorganized executor functions
  • Closed port 8080
  • Removed all function volumes (docker.sock, tmp, and functions storage)
  • Removed mount: /var/run/docker.sock from dev mode compose file on the appwrite container
  • Removed unused env vars from the functions worker

TODO:

  • Move build stage to a dedicated worker that could be reused later
  • Fix executor HTTP API to be REST compatible with proper usage of HTTP CRUD methods and entities as endpoint names.
  • Double check new indexes and if we miss any
  • When we delete a tag we should delete all builds
  • Should all v1/functions/builds be nested under /v1/functions/:id/tags/:id/builds or just under /v1/functions/:id?
  • Clean up functions that have not been executed for X INTERVAL
  • Have a maximum amount of runtimes that can co-exist.

Test Plan

N/A

Related PRs and Issues

N/A

Have you read the Contributing Guidelines on issues?

Yes

@christyjacob4
Copy link
Contributor

christyjacob4 commented Jan 24, 2022

Move build stage to a dedicated worker that could be reused later

Currently being worked on here . #2675

Fix executor HTTP API to be REST compatible with proper usage of HTTP CRUD methods and entities as endpoint names.

For the executor, I'm thinking of the following structure

Execute a function
POST /v1/executor/functions/:functionId

Delete a function
DELETE /v1/executor/functions/:functionId

Delete a tag
DELETE /v1/executor/functions/:functionId/tags/:tagId

Create Runtime Server
POST /v1/executor/functions/:functionId/tags/:tagId/runtime

Re-trigger a build
POST /v1/executor/tag/:tagId/builds/:buildId

Double check new indexes and if we miss any

Will Do ✅

When we delete a tag we should delete all builds

@eldadfux in the current structure, a build does not store the tagID it's associated with. We need to add a new tagId attribute for the builds collection so we're able to find all the associated builds for a tag easier.

Should all v1/functions/builds be nested under /v1/functions/:id/tags/:id/builds or just under /v1/functions/:id?

I personally prefer /v1/functions/:id/tags/:id/builds
In this case again, we still need to add a tagId attribute to each build so we're able to fetch them easily.

This was referenced Jan 24, 2022
@TorstenDittmann
Copy link
Contributor

Fixed all the conflicts with the target branch, tests pass locally 👍🏻

@christyjacob4 christyjacob4 merged commit 342f22f into feat-functions-refactor Jan 25, 2022
@stnguyen90 stnguyen90 deleted the feat-fx-review branch February 14, 2023 01:06
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.

4 participants