feat: add garbage-collect api for docker hosted registries #46
Conversation
Reviewer's Guide by SourceryThis pull request introduces a garbage collection feature for Docker-hosted registries, allowing users to initiate the process directly from the UI. It also refactors the Docker service to use the HTTP API, improving configuration flexibility. A new API endpoint Sequence diagram for Garbage CollectionsequenceDiagram
actor User
participant Browser
participant API Endpoint
participant DockerService
participant DockerRegistry
User->>Browser: Clicks 'Garbage Collect' button
Browser->>API Endpoint: POST /api/garbage-collect
API Endpoint->>DockerService: Calls runGarbageCollection()
DockerService->>DockerRegistry: Creates exec instance
DockerService->>DockerRegistry: Starts exec instance
DockerService->>DockerRegistry: Gets exec stream
DockerService->>DockerRegistry: Inspects exec instance
DockerRegistry-->>DockerService: Returns result
DockerService-->>API Endpoint: Returns success/failure
API Endpoint-->>Browser: Returns JSON response
Browser-->>User: Displays notification
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @kmendell - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a configuration option to enable/disable the garbage collection feature.
- Ensure that the Docker API version is configurable and validated.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| Pid: number; | ||
| } | ||
|
|
||
| interface ExecStreamResponse { |
There was a problem hiding this comment.
issue (complexity): Consider simplifying getExecStream by using an async iterator to handle the streaming data, which improves readability and reduces nested callbacks.
You can simplify getExecStream by replacing the nested event listeners with an async iterator over the streaming data. This reduces nested callbacks and improves readability while keeping the same functionality.
For example, change the method to this:
private async getExecStream(execId: string): Promise<ExecStreamResponse> {
try {
const response = await axios.post(
`${this.baseUrl}/exec/${execId}/start`,
{ Detach: false, Tty: false },
{ ...this.getAxiosConfig(), responseType: 'stream' }
);
let output = '';
for await (const chunk of response.data) {
output += chunk.toString();
}
// Note: Errors in the stream will cause the iteration to throw
return { output, error: '' };
} catch (error) {
logger.error('Failed to get exec stream:', error);
throw error;
}
}This approach uses the async iterator of the stream, eliminating the manual event listener setup and reducing cognitive overhead.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @kmendell - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a health check endpoint to verify the Docker daemon is accessible before attempting garbage collection.
- It would be useful to include the garbage collection logs in the UI for better transparency.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| Pid: number; | ||
| } | ||
|
|
||
| interface ExecStreamResponse { |
There was a problem hiding this comment.
issue (complexity): Consider using async iteration to handle the stream in getExecStream instead of manually wrapping it in a Promise.
Consider refactoring the manual Promise wrapping in getExecStream using async iteration on the stream. This reduces nesting and clarifies control flow. For example:
private async getExecStream(execId: string): Promise<ExecStreamResponse> {
try {
const response = await axios.post(
`${this.baseUrl}/exec/${execId}/start`,
{
Detach: false,
Tty: false,
},
{
...this.getAxiosConfig(),
responseType: 'stream',
}
);
let output = '';
// Async iterate over stream chunks
for await (const chunk of response.data) {
output += chunk.toString();
}
return { output, error: '' };
} catch (error) {
logger.error('Failed to get exec stream:', error);
throw error;
}
}This refactoring leverages the built-in async iterator support on streams and removes the need for manually handling data, end, and error events, making the code cleaner and easier to maintain.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
🐳 Docker image built and pushed! Image: You can pull and run this image with: docker pull ghcr.io/kmendell/svelocker-ui:pr-46
docker run -p 3000:3000 ghcr.io/kmendell/svelocker-ui:pr-46 |
|
🐳 Docker image built and pushed! Image: You can pull and run this image with: docker pull ghcr.io/kmendell/svelocker-ui:pr-46
docker run -p 3000:3000 ghcr.io/kmendell/svelocker-ui:pr-46 |
Summary by Sourcery
This pull request adds a garbage collection feature to the application, allowing users to initiate garbage collection on Docker-hosted registries. It also refactors the Docker service to use the HTTP API, providing more configuration options and improving compatibility.
New Features:
Enhancements:
Build:
Documentation: