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

Feat skeleton build task #4248

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Feat skeleton build task #4248

merged 2 commits into from
Oct 5, 2023

Conversation

drewbo
Copy link
Contributor

@drewbo drewbo commented Sep 11, 2023

Changes proposed in this pull request:

Extreme WIP (but functional) PR which implements the following items from the acceptance criteria

  • Create a build task queue
  • Run build task when a site build starts and when site build completes, depending on user/task config
  • Run task from worker thread
  • Update tasks queue with new status
  • On task complete, save task artifact and complete tasks queue with finish status.

I've also added a "hidden admin page" at /tasks which shows some build task information. The build task does run code, but currently it's fully encapsulated in the worker, rather than running a CF task. I have a number of problems with the current architecture and would love advice on proceeding:

  • As we've identified previously, the project has an odd mix of object-oriented vs functional programming throughout. The place it is most obvious here is the faux-instance methods enqueue (previously existing) and startBuildTask (added).
  • As written, the tasks are all implemented as worker code only, rather than CF tasks. They also have implied customs with no way to enforce them (function arguments and artifacts location)
  • Because the worker tasks have access to the DB directly, they don't necessary need to use the status endpoint but I've included it as an example. Do we want to have some tasks available as worker code only or should they all be CF tasks?

security considerations

New non-user enforced endpoint is added for status reporting (uses secret token to validate responses)

@drewbo drewbo changed the title Feat skeleton build task [WIP] Feat skeleton build task Sep 11, 2023
@drewbo drewbo force-pushed the feat-skeleton-build-task branch from eb74058 to de39abf Compare September 11, 2023 16:36
@drewbo drewbo requested a review from a team September 13, 2023 19:27
Copy link
Contributor

@apburnes apburnes left a comment

Choose a reason for hiding this comment

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

Mostly looking for your thoughts on different options for kicking off the build tasks. Once we test out running jobs on a worker, cf task, or third party service, we'll have a better idea of how we want the job processor interface to function. We may want to lean more on bullmq's API for managing the job lifecycle but this is a great start. Awesome work.

@drewbo drewbo force-pushed the feat-skeleton-build-task branch from c952d3d to 9c16540 Compare September 18, 2023 18:11
@drewbo
Copy link
Contributor Author

drewbo commented Sep 18, 2023

@apburnes updated in response to your feedback:

  • I added the Promise.allSettled to the tasks but right now there isn't a return value, seems okay just noting
  • it's currently not started the "build complete" build task: still looking into that

@drewbo drewbo force-pushed the feat-skeleton-build-task branch from b70611e to bdf1d4e Compare September 18, 2023 21:05
@drewbo drewbo requested a review from apburnes September 18, 2023 21:07
@apburnes
Copy link
Contributor

@drewbo looks good so far.

@drewbo drewbo changed the title [WIP] Feat skeleton build task Feat skeleton build task Sep 19, 2023
@drewbo drewbo force-pushed the feat-skeleton-build-task branch from 730fa0d to 777b036 Compare September 28, 2023 16:23
@drewbo drewbo mentioned this pull request Oct 2, 2023
1 task
@drewbo drewbo force-pushed the feat-skeleton-build-task branch from 8ae4684 to 89fc934 Compare October 3, 2023 18:20
@drewbo
Copy link
Contributor Author

drewbo commented Oct 3, 2023

This is ready to go again, in conjunction with cloud-gov/pages-cf-build-tasks#1. It contains API tests but nothing for the CF API call to the tasks endpoint; happy to add that before final review.

Also because this is a large feature, I'm available to demo or add more docs as needed.

@drewbo drewbo force-pushed the feat-skeleton-build-task branch from 89fc934 to 1b2e73b Compare October 5, 2023 14:48
@apburnes apburnes requested a review from a team October 5, 2023 17:10
Copy link
Contributor

@apburnes apburnes left a comment

Choose a reason for hiding this comment

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

👏

@drewbo drewbo merged commit 676e150 into main Oct 5, 2023
@drewbo drewbo deleted the feat-skeleton-build-task branch October 5, 2023 17:56
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.

2 participants