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(all commands): --service-name flag should have priority. #1264

Merged
merged 3 commits into from
Aug 1, 2024

Conversation

kpfleming
Copy link
Contributor

When the --service-name flag was added it was given the lowest
priority for obtaining a service ID. If a user supplied this flag but
also supplied --service-id, or had a fastly.toml with a service ID in
it, or had the FASTLY_SERVICE_ID environment variable set, the
--service-name flag would be ignored. This resulted in a user
accidentally deleting a service because the flag was ignored.

This commit changes the priority so that --service-name and
--service-id have equal (top) priority, and also ensures that
supplying both will result in an error. It also adds a unit test for
the argparser.ServiceID() function which implements this logic.

Closes #1261.

Rename FlagServiceDesc to FlagServiceNameDesc to be consistent with
FlagServiceID/FlagServiceIDDesc.
When the --service-name flag was added it was given the lowest
priority for obtaining a service ID. If a user supplied this flag but
also supplied --service-id, or had a fastly.toml with a service ID in
it, or had the FASTLY_SERVICE_ID environment variable set, the
--service-name flag would be ignored. This resulted in a user
accidentally deleting a service because the flag was ignored.

This commit changes the priority so that --service-name and
--service-id have equal (top) priority, and also ensures that
supplying both will result in an error. It also adds a unit test for
the argparser.ServiceID() function which implements this logic.

Closes fastly#1261.
Copy link
Member

@jsocol jsocol left a comment

Choose a reason for hiding this comment

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

lgtm! appreciate the clarity of --service-id vs --service-name

@kpfleming kpfleming merged commit 5075829 into fastly:main Aug 1, 2024
6 checks passed
@kpfleming kpfleming deleted the issue-1261 branch August 1, 2024 16:07
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.

[BUG] accidental service deletion when PR is cleaned up for failed deployment
2 participants