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.

Sign up for GitHub

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

Jump to bottom

clp-package: Add log-viewer-webui as a component. #476

Merged
merged 14 commits into from
Jul 12, 2024
Merged

clp-package: Add log-viewer-webui as a component. #476

merged 14 commits into from
Jul 12, 2024

Conversation

haiqi96
Copy link
Contributor

@haiqi96 haiqi96 commented Jul 9, 2024 •
edited
Loading

Description

This PR adds log_viewer as a component into the CLP package. The PR consists of three small changes.

  1. Adds the log_viewer into Taskfile.yml
  2. Packages node-22 into the package and distinguish it from the node-14
  3. Updates the start-clp script to launch log_viewer

Validation performed

Built a clp package and launched log_viewer

  1. Confirmed that the container doesn't generate any error message
  2. Tested with example route http://127.0.0.1:3000/examples/get/Alice
  3. Started and stopped the log viewer webui with start-clp log_viewer_webui and stop-clp log_viewer_webui

@haiqi96 haiqi96 marked this pull request as ready for review July 9, 2024 20:35
junhaoliao
junhaoliao requested changes Jul 10, 2024
View reviewed changes
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.

Many thanks for quickly coming up with this PR and all the insightful offline discussions.

Let's see if the suggestions make sense.

Taskfile.yml Outdated Show resolved Hide resolved
Taskfile.yml Outdated Show resolved Hide resolved
Taskfile.yml Outdated Show resolved Hide resolved
Taskfile.yml Show resolved Hide resolved
Taskfile.yml Outdated Show resolved Hide resolved
Taskfile.yml Outdated Show resolved Hide resolved
components/clp-package-utils/clp_package_utils/scripts/start_clp.py Show resolved Hide resolved
components/clp-package-utils/clp_package_utils/scripts/start_clp.py Show resolved Hide resolved
Taskfile.yml Show resolved Hide resolved
components/clp-py-utils/clp_py_utils/clp_config.py Outdated Show resolved Hide resolved
@junhaoliao junhaoliao self-requested a review July 12, 2024 18:09
junhaoliao
junhaoliao previously approved these changes Jul 12, 2024
View reviewed changes
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.

Looks good to me. I tested locally with task and launching the complete CLP package; then with Postman I was able to get a valid response from localhost:3000/examples/get/hi .

Taskfile.yml Outdated Show resolved Hide resolved
Taskfile.yml Outdated Show resolved Hide resolved
@haiqi96 haiqi96 changed the title clp-package: Add log viewer into the package clp-package: Add log viewer webui as a component of the clp package. Jul 12, 2024
junhaoliao
junhaoliao approved these changes Jul 12, 2024
View reviewed changes
Copy link
Member

@junhaoliao junhaoliao left a comment •
edited
Loading

Choose a reason for hiding this comment

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

How about clp-package: Add component log-viewer-webui. as the squashed commit message.

@haiqi96 haiqi96 changed the title clp-package: Add log viewer webui as a component of the clp package. clp-package: Add log-viewer-webui as a component. Jul 12, 2024
@haiqi96 haiqi96 merged commit 94d96d3 into y-scope:main Jul 12, 2024
4 checks passed
@Henry8192 Henry8192 mentioned this pull request Jul 12, 2024
Merged
jackluo923 pushed a commit to jackluo923/clp that referenced this pull request Dec 4, 2024
@haiqi96
clp-package: Add log-viewer-webui as a component. (y-scope#476)
d9bb90c
@haiqi96 haiqi96 deleted the log_viewer_package branch December 6, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kirkrodrigues kirkrodrigues kirkrodrigues left review comments

@junhaoliao junhaoliao junhaoliao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@haiqi96 @kirkrodrigues @junhaoliao