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

fix: admin and cli fixes #1922

Merged
merged 5 commits into from
Dec 2, 2024
Merged

fix: admin and cli fixes #1922

merged 5 commits into from
Dec 2, 2024

Conversation

jonaro00
Copy link
Member

No description provided.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added garbage collection commands and improved project management functionality in the admin CLI, focusing on resource cleanup and project lookup.

  • Added Gc and GcShuttlings commands in /admin/src/args.rs for cleaning up free tier projects and shuttlings based on time parameters
  • Added get_project_by_name_beta method in /api-client/src/lib.rs for retrieving projects by name instead of ID
  • Removed legacy IdleCch command from admin functionality

5 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

Minor fix to project name handling in the beta platform API client, removing unnecessary reference operator when passing the name parameter to get_project_by_name_beta().

  • Modified cargo-shuttle/src/lib.rs to pass name directly instead of &name when calling get_project_by_name_beta()
  • Change improves ownership semantics since API client expects to take ownership of the name parameter
  • Aligns with beta platform's project lookup functionality

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

Added implementation of garbage collection functionality and deployment stopping in the admin client, with rate limiting considerations.

  • Added stop_project_deployments helper in /admin/src/lib.rs with 1-second delay between API calls to prevent rate limiting
  • Implemented gc_free_tier and gc_shuttlings methods in /admin/src/client.rs to handle garbage collection API requests
  • Simplified /admin/src/main.rs by moving command handling to a separate run function for better code organization

5 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added no-follow deployment option and improved garbage collection implementation in the admin CLI.

  • Added no_follow flag in cargo-shuttle/src/lib.rs to allow skipping deployment status tracking
  • Added 5-second delay before starting garbage collection in /admin/src/lib.rs to prevent accidental project stops
  • Added rate limiting sleep between project stops in garbage collection to prevent API overload

5 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Based on the latest changes and previous reviews, here's my summary of the recent updates:

Improved error handling and rate limiting in admin CLI garbage collection functionality.

  • Added proper error propagation in /admin/src/lib.rs by replacing unwrap() calls with proper error handling
  • Fixed grammar in user-facing messages for garbage collection commands
  • Added missing documentation for RenewCerts command in /admin/src/args.rs

The changes focus on improving code quality and user experience while maintaining the core functionality added in previous iterations.

5 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

Improved websocket connection handling in device authentication flow with better error management and resource cleanup.

  • Added error propagation for ping message failures in cargo-shuttle/src/lib.rs, breaking the loop on send errors
  • Implemented proper cleanup by aborting pinger task after authentication completes
  • Added safeguards against potential resource leaks in websocket connection failures

1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

Removed API key validation check in project initialization flow, potentially impacting authentication error handling.

  • Removed API key requirement check in cargo-shuttle/src/lib.rs when creating Shuttle environments during project initialization
  • Change could lead to less clear error messages if API operations are attempted without authentication
  • Affects the project initialization flow's error handling and user feedback

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

Added a limit parameter to garbage collection commands in the admin CLI to control the maximum number of projects processed at once.

  • Added limit parameter with default value of 100 to both Gc and GcShuttlings commands in /admin/src/args.rs
  • Implemented truncation of project IDs based on limit parameter in /admin/src/lib.rs gc function
  • Added project count display in output for better visibility of affected resources

2 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

@jonaro00 jonaro00 merged commit 5431b1f into main Dec 2, 2024
29 of 33 checks passed
@jonaro00 jonaro00 deleted the cch24 branch December 2, 2024 23:23
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.

2 participants