Skip to content

Add detection for incompatible flags with --run-file#1509

Merged
yaron2 merged 5 commits into
dapr:masterfrom
twinguy:master
May 6, 2025
Merged

Add detection for incompatible flags with --run-file#1509
yaron2 merged 5 commits into
dapr:masterfrom
twinguy:master

Conversation

@twinguy

@twinguy twinguy commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

Description

This PR adds detection for incompatible flags when using the --run-file flag in the dapr run command. When a user tries to use command line flags that are incompatible with the run file configuration, a warning message is now displayed to inform the user that these flags will be ignored and should instead be configured in the run file.

Issue reference

Please reference the issue this PR will close: #1508

Changes implemented

  • Added a list of flag names that are incompatible with the --run-file option
  • Implemented a detection function (detectIncompatibleFlags) that identifies when incompatible flags are being used
  • Created a message generator (generateWarningMessage) to create a clear warning for users
  • Added logic in the run command to check for incompatible flags and display warnings
  • Added unit tests for both the detection function and message generator

Behavior change

Before: When using both --run-file and other command-line flags that should be configured in the run file, the command-line flags were silently ignored, potentially causing confusion for users.

After: When using both --run-file and incompatible command-line flags, a warning message is displayed to inform the user that these flags are ignored and should be configured in the run file.

Example warning

WARNING: The following flags are ignored when using --run-file and should be configured in the run file instead: app-id, app-port, dapr-http-port

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@twinguy
twinguy requested review from a team as code owners March 24, 2025 04:47
antontroshin
antontroshin reviewed Apr 10, 2025
View reviewed changes
Comment thread cmd/run.go Outdated
Comment thread cmd/run.go
@twinguy
twinguy requested a review from antontroshin April 12, 2025 15:20
@antontroshin

antontroshin commented Apr 13, 2025

Copy link
Copy Markdown
Contributor

@twinguy Thank you for the contribution and the quick fix.
One small thing left is, seems like CI step dapr_cli / Build linux_amd64 binaries (pull_request) fails on go mod tidy check.
Could you please run go mod tidy command locally on this branch and push the changes?
Other than that, LGTM!

@twinguy

twinguy commented Apr 16, 2025

Copy link
Copy Markdown
Contributor Author

@antontroshin Should be all good with latest "go mod tidy". This is my first contribution, so unsure what next steps are for getting final review/merge completed are. Thanks for all the great feedback and help.

@yaron2
yaron2 merged commit b51eab0 into dapr:master May 6, 2025
@yaron2 yaron2 added this to the v1.16 milestone May 6, 2025
@yaron2

yaron2 commented May 6, 2025

Copy link
Copy Markdown
Member

@twinguy thanks for your contribution

@marcduiker

marcduiker commented Jun 11, 2025

Copy link
Copy Markdown
Contributor

@holopin-bot @twinguy Thank you! Here's a digital badge as a small token of appreciation.

@holopin-bot

holopin-bot Bot commented Jun 11, 2025

Copy link
Copy Markdown

Congratulations @twinguy, the maintainer of this repository has issued you a badge! Here it is: https://holopin.io/claim/cmbrz7mfw185707lacioonl84

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@antontroshin antontroshin antontroshin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@twinguy twinguy

Labels

None yet

Projects

None yet

Milestone

v1.16

Development

Successfully merging this pull request may close these issues.

dapr run arguments are not working when used with --run-file

4 participants

@twinguy @antontroshin @yaron2 @marcduiker