Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the apply command to the toolbox, enabling users to apply YAML configurations to the server via a REST API. It also updates the admin server to return a 204 No Content status upon successful resource updates. Key feedback includes fixing a format specifier bug in the URL construction, refactoring file handling to use defer for safer resource cleanup, and correcting a test case that was mistakenly targeting the serve command instead of the apply command.
|
/gemini review |
6e28433 to
6b31505
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces the apply command to the toolbox CLI, enabling users to synchronize local YAML configuration files with the toolbox server. The implementation includes logic for fetching existing primitives, comparing them to local definitions, and updating them via the admin API. Feedback identifies a critical bug in the applyPrimitive function where the HTTP method is incorrectly set to GET and the request body structure is missing a required config wrapper. Additionally, it is recommended to avoid http.DefaultClient in favor of a client with a defined timeout and to address potential type mismatches when using reflect.DeepEqual on data unmarshaled from different formats (JSON vs. YAML).
Description: apply configuration to the server
To load or update configuration files, after making changes, users can run toolbox apply. Dynamic reloading capability within Toolbox will be triggered when toolbox apply is run.
If a user modifies their configuration file directly, it will not trigger dynamic reloading of the Toolbox server. This is different from the existing Toolbox implementation that will automatically update the server when changes are detected in the configuration file. With the control plane, users have to run
toolbox applyafter making changes in order for it to take effect.The introduction of the
toolbox applycommand changes how server updates are handled; simply editing the configuration file will no longer cause the server to reload dynamically. Instead, users must now explicitly initiate the dynamic reloading process by executingtoolbox applyafter modifying their configuration. This new workflow ensures that server updates remain seamless and do not require a full restart.This subcommand will run the following:
Retrieve the Current State
Before making any changes, the CLI should send a GET request to the active server's Admin API.
This retrieves the "current state," which is the configuration of all existing primitives currently running on the server.
Store this state in an in-memory dictionary or map, keyed by the primitive's Kind and Name.
Parse the Desired State
Read and parse the local YAML configuration files or directories passed via the toolbox apply -f command.
Consolidate all parsed primitives into a "desired state" map, also keyed by Kind and Name.
Calculate the Delta (The Union Strategy)
The design dictates that the CLI must correctly calculate the union of the newly applied files and the existing primitives on the server.
Iterate through the desired state and current state maps to categorize the required HTTP requests:
New primitives (PUT): If a primitive exists in the desired state (local files) but not in the current state (server), mark it for creation via a PUT request.
Updating primitives (PUT): If a primitive exists in both states, perform a deep comparison (such as a field-by-field check or a configuration hash). If the configuration has changed, mark it for an update via a PUT request.
Ignore updates (Prevent Unintended Deletions): If a primitive exists in the current state (server) but is missing from the desired state (local files), take no action. Do not mark it for deletion. Ignoring missing files allows users to apply a single new configuration file without wiping out the rest of the server's active primitives. All deletions should be strictly deferred to the explicit toolbox delete command. This operational logic mirrors the Kubernetes apply subcommand; for scenarios where explicit deletion is required, users may specify a
--pruneflag.Execute API Requests for Dynamic Reloading
Once the delta is calculated, act as an HTTP client and push the necessary PUT requests to the deployed server's endpoint. If there are multiple primitives being updated at once, always run the source before tools and toolsets.
To prevent full server restarts or downtime, the server-side logic must handle these incoming requests by dynamically reloading only the specific primitive being updated.