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

add config:clear subcommand #377

Closed
wants to merge 2 commits into from
Closed

add config:clear subcommand #377

wants to merge 2 commits into from

Conversation

benjodo
Copy link
Member

@benjodo benjodo commented Mar 7, 2025

@benjodo benjodo requested a review from UserNotFound as a code owner March 7, 2025 02:55
@UserNotFound
Copy link
Member

Any background on why this was requested or how it's intended to be used?

One gotcha would be that this implementation doesn't account for opeation concurrency:

  • An operation is running which is adding a new value aptible config:set --app foo NEW=value
  • Before that operation completes, someone runs aptible config:clear --app foo

The result is that the clears all but the NEW variable, because it wasn't none to the client at the time the operation was created, but will exist when it runs.

@benjodo
Copy link
Member Author

benjodo commented Mar 7, 2025

Closing this PR for now, as there are valid concerns about how this feature would be used, and the impacts of its use and unintended consequences. config:unset remains a safer approach for our customers at this time. There are scripting workarounds to unset all variables as needed, such as:

APTIBLE_OUTPUT_FORMAT=json aptible config --app APP_HANDLE --env ENV_HANDLE | jq -r '.[].key' | xargs  aptible config:rm --app APP_HANDLE --env ENV_HANDLE

@benjodo benjodo closed this Mar 7, 2025
@UserNotFound
Copy link
Member

Also, this deletes APTIBLE_DOCKER_IMAGE/APTIBLE_PRIVATE_REGISTRY_USERNAME/APTIBLE_PRIVATE_REGISTRY_PASSWORD which seems like a pretty big footgun.

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