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: docker widget #2288

Open
wants to merge 24 commits into
base: dev
Choose a base branch
from
Open

feat: docker widget #2288

wants to merge 24 commits into from

Conversation

hillaliy
Copy link
Contributor

@hillaliy hillaliy commented Feb 9, 2025


Homarr

Thank you for your contribution. Please ensure that your pull request meets the following pull request:

  • Builds without warnings or errors (pnpm buid, autofix with pnpm format:fix)
  • Pull request targets dev branch
  • Commits follow the conventional commits guideline
  • No shorthand variable names are used (eg. x, y, i or any abbrevation)

@hillaliy hillaliy self-assigned this Feb 9, 2025
@hillaliy hillaliy added the enhancement New feature or request label Feb 9, 2025
@hillaliy hillaliy linked an issue Feb 9, 2025 that may be closed by this pull request
Copy link

deepsource-io bot commented Feb 9, 2025

Here's the code health analysis summary for commits f19aa29..54023a0. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

github-actions bot commented Feb 9, 2025

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 22.62% 10209 / 45119
🔵 Statements 22.62% 10209 / 45119
🔵 Functions 29.79% 426 / 1430
🔵 Branches 64.53% 1223 / 1895
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/api/src/router/docker/docker-router.ts 64.19% 69.23% 100% 64.19% 23-56, 59-63, 67-72, 101, 115, 129, 143, 159-160, 177-178, 195-203
packages/definitions/src/widget.ts 100% 100% 100% 100%
packages/request-handler/src/docker.ts 47.22% 66.66% 0% 47.22% 23-26, 52-53, 65-67, 70-127
packages/widgets/src/index.tsx 50% 100% 0% 50% 70-86, 101-114
packages/widgets/src/docker/component.tsx 0% 0% 0% 0% 1-303
packages/widgets/src/docker/index.ts 100% 100% 50% 100%
Generated in workflow #5471 for commit 54023a0 by the Vitest Coverage Report Action

@hillaliy hillaliy marked this pull request as ready for review February 10, 2025 19:05
@hillaliy hillaliy requested a review from a team as a code owner February 10, 2025 19:05
@hillaliy
Copy link
Contributor Author

I will remove unused variables

@hillaliy
Copy link
Contributor Author

Screenshot 2025-02-10 at 21 03 07 Screenshot 2025-02-10 at 21 03 15

@manuel-rw
Copy link
Member

Hi @hillaliy , thanks for the contribution. Please see the lint and DeepSource first. After that, we'll happily review your PR. Thanks!

@hillaliy
Copy link
Contributor Author

Lint and deepSource errors is because unused variables. I'm still thinking if need to add options so if you can review my PR and if you have an idea for widget options. Thanks

hillaliy and others added 6 commits February 12, 2025 19:07
Co-authored-by: Crowdin Homarr <190541745+homarr-crowdin[bot]@users.noreply.github.com>
Co-authored-by: homarr-renovate[bot] <158783068+homarr-renovate[bot]@users.noreply.github.com>
@hillaliy
Copy link
Contributor Author

@Meierschlumpf I need your help with this widget. Refresh rate is not good enough.

@Meierschlumpf
Copy link
Member

  1. What do you mean with refresh rate?
  2. How can I help you?

@hillaliy
Copy link
Contributor Author

I mean with data refresh. Can you pull it and try it?

@Meierschlumpf
Copy link
Member

Okay yeah currently it is cached for 5 minutes:

const dockerCache = createCacheChannel<{
  containers: (ContainerInfo & { instance: string; iconUrl: string | null; cpuUsage: number; memoryUsage: number })[];
}>("docker-containers", 5 * 60 * 1000);

I would suggest that we replace this with the request-handlers we use for other integrations as well. Then we can also add a subscription functionallity

@ajnart
Copy link
Contributor

ajnart commented Feb 19, 2025

@hillaliy i think for this use case we'd want the data to be refreshed after the page loads as well as after every action (start/stop/restart)
Refreshing it in the background is not needed since the data is not shown on dashboards

@Meierschlumpf
Copy link
Member

It's the widget and not a modal or something, so we want to refresh the data every few seconds

@hillaliy hillaliy marked this pull request as draft March 1, 2025 07:11
@hillaliy hillaliy marked this pull request as ready for review March 5, 2025 16:40
@Meierschlumpf Meierschlumpf self-requested a review March 7, 2025 17:16
Copy link
Member

@Meierschlumpf Meierschlumpf left a comment

Choose a reason for hiding this comment

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

Also there should be a background job that periodically updates the data (like every minute) and we need to decide who should be able to add this widget to a board. The interactions should be restricted to only admins I would guess.

The cqmin system was just removed, so please adjust that as well

@manuel-rw manuel-rw self-requested a review March 7, 2025 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

feat: widget for docker containers stats
4 participants