feat(new-webui): MongoCDC Framework#809
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
junhaoliao
left a comment
There was a problem hiding this comment.
nice work
i understand this PR is still WIP. Let's use the suggestions as a checklist before requesting for a formal review
There was a problem hiding this comment.
NOTE: changes in this file is to be removed.
| toArray () { | ||
| return new Promise((resolve, reject) => { | ||
| this.socket.emit("collection::find::toArray", { | ||
| query: this.findQuery, | ||
| options: this.findOptions, | ||
| }, (response: { error?: any; data?: any }) => { | ||
| if (response.error) { | ||
| return reject(response.error); | ||
| } | ||
|
|
||
| return resolve(response.data); | ||
| }); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
NOTE: not in use at the moment.
| /** | ||
| * @type {Listener} | ||
| */ | ||
| private listener: Listener | null = null; |
There was a problem hiding this comment.
For private properties / functions, can we use the #propertyName syntax?
| @@ -14,6 +14,11 @@ export default defineConfig({ | |||
| target: "http://localhost:3000/", | |||
There was a problem hiding this comment.
Since the comment in L12-13 applies to both routes:
- Let move the comment to Line 11
- Let say "target host and port" instead of "target" to improve clarity.
@coderabbitai Let's create an issue to track the effort of serving Fastify routes under "/api". And we should replace this "/query/" proxy entry with "/api/" to ensure the other RESTful APIs are proxied to the backend.
There was a problem hiding this comment.
🧩 Analysis chain
@junhaoliao I'll create an issue to track this effort. Let me take care of that for you.
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue to track the API routing changes
gh issue create \
--title "Replace '/query/' proxy with '/api/' for RESTful endpoints" \
--body "## Background
Currently, the log-viewer-webui client proxies API requests through the '/query/' path, but this doesn't account for other RESTful endpoints.
## Objective
- Serve all Fastify routes under the '/api/' path
- Update the Vite configuration to proxy '/api/' to the backend server instead of just '/query/'
## References
- Requested in PR: https://github.com/y-scope/clp/pull/809
- Discussion comment: https://github.com/y-scope/clp/pull/809#discussion_r2034462565
## Assignee
@junhaoliao" \
--assignee junhaoliao \
--repo y-scope/clp
Length of output: 718
@junhaoliao I've created an issue to track the effort of serving Fastify routes under "/api/" and replacing the "/query/" proxy entry. You can find it here: #810
The issue includes the objectives we discussed about updating the Vite configuration to ensure all RESTful APIs are properly proxied to the backend.
| /** | ||
| * Initialize the MongoDB replica set. | ||
| * | ||
| * @param {FastifyInstance} fastify | ||
| * @return {Promise<void>} | ||
| * @throws {Error} If the replica set initialization fails. | ||
| */ | ||
| const initializeReplicaSet = async (fastify: FastifyInstance): Promise<void> => { | ||
| try { | ||
| const directMongoClient = new MongoClient( | ||
| "mongodb://localhost:27017", | ||
| {replicaSet: "rs0", directConnection: true} | ||
| ); | ||
| const response = await directMongoClient.db("admin").admin() | ||
| .command({replSetInitiate: {}}); | ||
|
|
||
| fastify.log.info("Replica set initialized:", response); | ||
| } catch (e) { | ||
| if (e instanceof MongoServerError && "AlreadyInitialized" === e.codeName) { | ||
| return; | ||
| } | ||
| throw new Error("Failed to initialize replica set", {cause: e}); | ||
| } | ||
| }; |
There was a problem hiding this comment.
We can remove all references to this, since the clp-package should have done the init in some Python script when starting the package.
| /** | |
| * Initialize the MongoDB replica set. | |
| * | |
| * @param {FastifyInstance} fastify | |
| * @return {Promise<void>} | |
| * @throws {Error} If the replica set initialization fails. | |
| */ | |
| const initializeReplicaSet = async (fastify: FastifyInstance): Promise<void> => { | |
| try { | |
| const directMongoClient = new MongoClient( | |
| "mongodb://localhost:27017", | |
| {replicaSet: "rs0", directConnection: true} | |
| ); | |
| const response = await directMongoClient.db("admin").admin() | |
| .command({replSetInitiate: {}}); | |
| fastify.log.info("Replica set initialized:", response); | |
| } catch (e) { | |
| if (e instanceof MongoServerError && "AlreadyInitialized" === e.codeName) { | |
| return; | |
| } | |
| throw new Error("Failed to initialize replica set", {cause: e}); | |
| } | |
| }; | |
| /** | |
| * Initialize the MongoDB replica set. | |
| * | |
| * @param {FastifyInstance} fastify | |
| * @return {Promise<void>} | |
| * @throws {Error} If the replica set initialization fails. | |
| */ | |
| const initializeReplicaSet = async (fastify: FastifyInstance): Promise<void> => { | |
| try { | |
| const directMongoClient = new MongoClient( | |
| "mongodb://localhost:27017", | |
| {replicaSet: "rs0", directConnection: true} | |
| ); | |
| const response = await directMongoClient.db("admin").admin() | |
| .command({replSetInitiate: {}}); | |
| fastify.log.info("Replica set initialized:", response); | |
| } catch (e) { | |
| if (e instanceof MongoServerError && "AlreadyInitialized" === e.codeName) { | |
| return; | |
| } | |
| throw new Error("Failed to initialize replica set", {cause: e}); | |
| } | |
| }; |
|
@AVMatthews / @davemarco / @hoophalab Please voice any preferences if you feel the |
|
I looked briefly - does it make sense to keep this open as a WIP. Then have two smaller PRs for server and client. At almost ~1000, it may sense to split Also for syntax, we normally only Uppercase first letter in code. so MongoCdc |
Getting the stuff cleaned in this PR first then submitting separately is a good idea. By the way, I tend to avoid naming anything with "CDC" at this point because we're not currently handling (and for performance reasons even in the future we will not be always handling) the delta changes in the change packets. |
Description
New framework built on MongoCDC using socket.io meant to replace our current usage of meteor's publish-subscribe framework.
TODO:
Checklist
breaking change.
Validation performed
Manually added collection to mongo replica and tested to see that the contest of the collection could be requested through the client from the server.