Add schema proposal composition background job#7706
Conversation
Summary of ChangesHello @jdolle, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the system's ability to manage schema proposals by offloading the composition and validation process to an asynchronous background job. This architectural change improves responsiveness and scalability by preventing blocking operations during schema updates. It establishes the necessary infrastructure within the workflows service to interact with an external schema composition service, laying the groundwork for more robust and efficient schema management. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
🐋 This PR was built and pushed to the following Docker images: Targets: Platforms: Image Tag: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a background job for schema proposal composition, including a new workflow and schema provider. However, a critical pagination logic error in proposedSchemas can lead to an infinite loop, causing a Denial of Service (DoS) for background workers. Additionally, there's a risk of worker crashes due to unhandled empty arrays and a missing ownership check (IDOR). Addressing these critical issues, along with other identified data fetching problems, potential runtime errors, and opportunities for improved type safety and style guide adherence, is crucial before merging.
💻 Website PreviewThe latest changes are available as preview in: https://pr-7706.hive-landing-page.pages.dev |
| query: sql` | ||
| ALTER TABLE IF EXISTS "schema_proposals" | ||
| ADD COLUMN IF NOT EXISTS "composition_status" TEXT | ||
| , ADD COLUMN IF NOT EXISTS "composition_timestamp" TIMESTAMPTZ |
There was a problem hiding this comment.
Looking for feedback on this.
Should the composition state for the proposal be per-save or is it acceptable to be per-proposal (latest state only)?
There was a problem hiding this comment.
I think the current/latest state is the only relevant composition state in a proposal.
n1ru4l
left a comment
n1ru4l
left a comment
There was a problem hiding this comment.
Gonna approve but please take care of two things
- changeset for self-hosters and documentation of the new env variables in the readme
- use the TaskScheduler class for scheduling the task on API side
3 participants
Background
Schema proposals need to run composition when they're updated. By creating a workflow, composition can be ran in the background on proposal update.
Description
This adds a background job to run composition when proposals are updated, and adds more columns to the schema proposal record to store the latest composition state.
A subscription was added to allow async updating of the composition state but the frontend logic for the subscription is not yet added because this PR is getting large.
Also, the redis pubsub instance was factored out into a separate package to avoid a circular dependency between the workflows and api packages.
Checklist