-
Notifications
You must be signed in to change notification settings - Fork 202
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 capability to run to support -f
flag
#1161
Conversation
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Codecov Report
@@ Coverage Diff @@
## master #1161 +/- ##
==========================================
- Coverage 31.67% 26.40% -5.27%
==========================================
Files 36 38 +2
Lines 2526 2579 +53
==========================================
- Hits 800 681 -119
- Misses 1630 1832 +202
+ Partials 96 66 -30
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
-f
flag-f
flag
assert.Contains(t, output, "failed to load components: open ../testdata/nonexistentdir:") | ||
assert.Contains(t, output, "Exited App successfully") | ||
assert.Contains(t, output, "Exited Dapr successfully") | ||
require.Error(t, err, "run did not fail") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
run must fail if a non existent dir is given.
@@ -169,6 +171,7 @@ dapr run --app-id myapp --dapr-path /usr/local/dapr | |||
print.FailureStatusEvent(os.Stderr, err.Error()) | |||
os.Exit(1) | |||
} | |||
// TODO: In future release replace following logic with the refactored functions seen below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refactor of actual run will be done after the initial release for run -f
feature.
pkg/print/print.go
Outdated
if logAsJSON { | ||
logJSON(w, string(status), fmt.Sprintf(fmtstr, a...)) | ||
} else { | ||
if (w == os.Stdout || w == os.Stderr) && runtime.GOOS != windowsOS { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this check so that, in future, normal dapr run outputing to stdout can automatically use this StatusEvent function. This is the function used in the individual functions inside cmd/run.go
@@ -1,5 +1,5 @@ | |||
/* | |||
Copyright 2021 The Dapr Authors | |||
Copyright 2023 The Dapr Authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just moves the pkg/standalone/run_test.go file into this package and modifies the imports for that.
dirPath := config.ResourcesPath | ||
if dirPath == "" { | ||
dirPath = config.ComponentsPath | ||
} | ||
_, err := os.Stat(dirPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the function that is introducing the E2E change seen in this PR. Previously resources path value was not validated. Now it is being validated.
} | ||
|
||
// RunOutput represents the run execution. | ||
type RunOutput struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will later be removed, once the normal dapr run
also migrates to using the RunExec
struct.
return nil | ||
} | ||
|
||
func NewOutput(config *standalone.RunConfig) (*RunOutput, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exactly same as pkg/standalone/run.go#Run function. moved here and renamed.
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
2810e00
to
63afff6
Compare
@mukundansundar Multiple attempt to use |
Will add it as a change in the follow up E2E PR. |
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
Resolved the above comments with the following summary: Changes that will be done in follow-up PRs
Changes planned for next release
|
@artursouza the changes are done. Please review |
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Follow up work items mentioned in this comment -#1161 (comment)
Signed-off-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com>
-f
flag-f
flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple of nits. But LGTM
Description
This adds the initial commits and changes for
dapr run -f
.Renames
Apps
asApp
struct.Refactors run in different functions without modifying existing Run code.
Adds code to execute run on multiple apps, with ability to write logs to files as needed.
LIMITATIONS
dapr run -f
command, the CLI process will still remain running waiting for a SIGINT.That is for adding red color to the text when viewed in shell.
FOLLOW-UP WORK
Precendence PR merge- Precedence rule for resources directory, config file and envs #1158Generating proper defaults to be used by daprd- done in set default fields via struct default tag #1162Setting env var for the commands- Part of Precedence rule for resources directory, config file and envs #1158Output of
dapr run -f dapr.yaml
:Output of
dapr list
:daprd Logs for processor
app logs for processor
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
related to #1143
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: