[SC-35118] Support deployment without overloaded environment variables. #396
Conversation
UserNotFound
force-pushed
the
app-deploy-settings
branch
from
c7b5ff5 to
206c94a
Compare
UserNotFound
force-pushed
the
app-deploy-settings
branch
from
c2d72d0 to
1eeb806
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR migrates deployment functionality from using overloaded environment variables to using dedicated settings and sensitive_settings parameters for the Operation API. This supports a cleaner separation between user environment variables and deployment configuration.
Key Changes:
- Deprecated passing deployment configuration as environment variables (e.g.,
APTIBLE_DOCKER_IMAGE=value) - Migrated to use
settingsfor non-sensitive deployment configuration andsensitive_settingsfor credentials - Updated CLI options to explicitly define each parameter instead of programmatically generating them
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/aptible/cli/subcommands/deploy.rb | Refactored to use settings and sensitive_settings parameters instead of environment variables; deprecated old environment variable pattern |
| spec/aptible/cli/subcommands/deploy_spec.rb | Updated tests to expect settings and sensitive_settings parameters; changed expectations for deprecated environment variable usage |
| lib/aptible/cli/version.rb | Bumped version to 0.25.2 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
UserNotFound
requested review from
almathew and
madhuravius
and removed request for
almathew
UserNotFound
requested review from
aguilinger
and removed request for
madhuravius
| m = 'ERROR: The environment variables ' \ | ||
| "#{DEPRECATED_ENV.join(', ')} are deprecated. " \ | ||
| "Use the options #{option_names}, instead" | ||
| raise Thor::Error, m |
There was a problem hiding this comment.
are we going straight to error and not doing a warning first?
There was a problem hiding this comment.
I can change to a warning for now.
| module Aptible | ||
| module CLI | ||
| VERSION = '0.25.1'.freeze | ||
| VERSION = '0.25.2'.freeze |
There was a problem hiding this comment.
if we are hard erroring on previously fine inputs, probably deserves more than a patch bump
Deploy using Operation.settings and Operation.sensitive_settings
This can go out after release of https://github.com/aptible/sweetness/pull/2024 (already relased)
It works seemlessly in all phases of migrating to settings.
It is not required for customers to us this update until phase 3.
The user experience/interface for the deploy command is not changed at all.
It's implicitly tested in integration (given we have
aptible deploy --docker-imagetested), but also explicitly tested extensively in the API-based deployment specs, which are the same types of Operation parameters this CLI command could/would create.