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

log-viewer-webui: Add server implementation for submitting IR extraction jobs and serving IR files; Don't copy unnecessary files when building component. #458

Merged
merged 56 commits into from
Jul 20, 2024

Conversation

wraymo
Copy link
Contributor

@wraymo wraymo commented Jun 24, 2024

Description

This PR adds a basic server implementation for log-viewer-webui.

  • It includes a DbManager to handle connections for both MySQL and MongoDB.
  • It adds routes for querying and inserting data.

Validation performed

  • Built and started the clp package, added CLP_DB_PASS and CLP_DB_USER to .env file.
  • Ran npm test.

Copy link
Member

@junhaoliao junhaoliao left a comment

Choose a reason for hiding this comment

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

Thanks a lot for picking up my backlog. Let's update some of the implementations after #460 is merged.

components/log-viewer-webui/server/src/DbManager.js Outdated Show resolved Hide resolved
components/log-viewer-webui/server/src/DbManager.js Outdated Show resolved Hide resolved
components/log-viewer-webui/server/src/DbManager.js Outdated Show resolved Hide resolved
fastify.post("/examples/post", async (req, resp) => {
return {msg: `Goodbye, ${req.body.name}!`};
fastify.post("/decompression_job", async (req, resp) => {
const result = await fastify.dbManager.insertDecompressionJob(req.body);
Copy link
Member

Choose a reason for hiding this comment

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

Can we validate the parameters before passing it to the db manager? / Can we simply accept a job id from the request body?

@@ -6,12 +6,20 @@
* @return {Promise<void>}
*/
const routes = async (fastify, options) => {
Copy link
Member

Choose a reason for hiding this comment

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

I feel it might be better keeping the examples here. (Those can serve as heart beat / echo routes whenever we need check whether the server is online for debug purposes. )

Can we keep the examples and move the decompression routes into another file like query.js or decompression.js?

components/log-viewer-webui/server/src/app.js Outdated Show resolved Hide resolved
Comment on lines 19 to 23
fastify.get("/stats", async (req, resp) => {
const result = await fastify.dbManager.getStats();
console.log(result);
resp.send(result);
});
Copy link
Member

Choose a reason for hiding this comment

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

This was added as an example route in WIP #434 before the Alice / Bob examples were added. Now we have the better example routes, maybe we don't need this route any more.

components/log-viewer-webui/server/src/app.test.js Outdated Show resolved Hide resolved
# Conflicts:
#	components/log-viewer-webui/server/package-lock.json
#	components/log-viewer-webui/server/package.json
#	components/log-viewer-webui/server/src/app.js
#	components/log-viewer-webui/server/src/main.js
@@ -1,3 +1,8 @@
CLIENT_DIR=../client/dist
IR_DATA_DIR=../../../build/clp-package/var/data/ir
LOG_VIEWER_DIR=../yscope-log-viewer/dist
Copy link
Member

Choose a reason for hiding this comment

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

@Henry8192 Kindly update here once your yscope-log-viewer integration PR is ready.

@@ -798,6 +799,7 @@ def start_log_viewer_webui(instance_id: str, clp_config: CLPConfig, mounts: CLPD
"--log-driver", "local",
"-e", f"NODE_PATH={node_path}",
"-e", f"CLIENT_DIR={container_log_viewer_dir}/client/dist",
Copy link
Member

Choose a reason for hiding this comment

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

We should also add LOG_VIEWER_DIR once that is ready.

await server.register(fastifyStatic, {
prefix: "/log-viewer",
root: logViewerDir,
decorateReply: false,
Copy link
Member

Choose a reason for hiding this comment

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

Needed for any second and after registration of fastifyStatic: https://github.com/fastify/fastify-static#disabling-reply-decorator

@junhaoliao junhaoliao requested a review from kirkrodrigues July 19, 2024 15:59
@junhaoliao junhaoliao requested a review from kirkrodrigues July 19, 2024 20:08
@junhaoliao junhaoliao requested review from kirkrodrigues and removed request for kirkrodrigues July 19, 2024 20:37
Copy link
Member

@kirkrodrigues kirkrodrigues left a comment

Choose a reason for hiding this comment

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

For the PR title, how about:

log-viewer-webui: Add server implementation for submitting IR extraction jobs and serving IR files; Don't copy unnecessary files when building component.

@junhaoliao junhaoliao changed the title log-viewer-webui: Add basic server implementation log-viewer-webui: Add server implementation for submitting IR extraction jobs and serving IR files; Don't copy unnecessary files when building component. Jul 20, 2024
@junhaoliao junhaoliao merged commit 921a388 into y-scope:main Jul 20, 2024
4 checks passed
jackluo923 pushed a commit to jackluo923/clp that referenced this pull request Dec 4, 2024
…ion jobs and serving IR files; Don't copy unnecessary files when building component. (y-scope#458)
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