delete design code #245
delete design code #245
Conversation
Codecov Report
@@ Coverage Diff @@
## main #245 +/- ##
==========================================
- Coverage 22.41% 13.69% -8.72%
==========================================
Files 35 46 +11
Lines 1593 2724 +1131
==========================================
+ Hits 357 373 +16
- Misses 1226 2339 +1113
- Partials 10 12 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| func TestMongoService_DeleteDesignCode(t *testing.T) { | ||
| mt := mtest.New(t, mtest.NewOptions().ClientType(mtest.Mock)) | ||
| defer mt.Close() | ||
| mt.Run("success", func(mt *mtest.T) { |
There was a problem hiding this comment.
Is this possible to add more than one code versions and delete only one of them and check if the specified version is only removed?
There was a problem hiding this comment.
we have just mocked mongo db responses here, I suggest we should do this as part of integration test.
cmd/flamectl/cmd/delete_code.go
Outdated
| ) | ||
|
|
||
| var removeCodeCmd = &cobra.Command{ | ||
| Use: "code <version>", |
There was a problem hiding this comment.
Is this usage format correct? What about -d argument? Can you double-check if this format is correct?
There was a problem hiding this comment.
-d argument was made as mandate MarkPersistentFlagRequired, so command will fail without this argument. Anyways added it in description.
There was a problem hiding this comment.
Thanks for this. I was asking if someone runs flamectl remove code -h, what would be a usage output?
I was asking if the usage is correctly printed out. Should -d be part of a usage description as it is mandatory?
cmd/flamectl/cmd/delete_code.go
Outdated
| var removeCodeCmd = &cobra.Command{ | ||
| Use: "code <version>", | ||
| Short: "Remove a code", | ||
| Long: "This command removes a code", |
There was a problem hiding this comment.
Can you add more description? A code should be associated with a design. So, a code should be removed from a design. Can you make the description a bit more informative?
cmd/flamectl/resources/code/code.go
Outdated
| @@ -72,7 +72,7 @@ func Create(params Params) error { | |||
| return nil | |||
| } | |||
|
|
|||
| fmt.Printf("Code created successfully for design %s\n", params.DesignId) | |||
| fmt.Printf("Code created successfully for design %s, version %v\n", params.DesignId, params.CodeVer) | |||
There was a problem hiding this comment.
When a code is created, user doesn't need to provide CodeVer. The DB code for creating a code will determine a version (https://github.com/cisco-open/flame/pull/245/files#diff-2164dd2c9fd70fa0f3feb28786e75a81fbe9feed581c045325da3c98c5a331e0R46-R47). So, here params.CodeVer will always print 0 or some arbitrary value.
We first need to check what response body returns and make sure that a code version is in the response body.
We may have to enhance this with a separate PR.
There was a problem hiding this comment.
we need to correct logs of flamectl in few other locations as well, we shall handle it is a part of different PR, for now removing this change.
| func (db *MongoService) DeleteDesignCode(userId string, designId string, version string) error { | ||
| zap.S().Debugf("delete design code 1: %v, %v,%v", userId, designId, version) | ||
|
|
||
| if version == "" || version == latestVersion { |
There was a problem hiding this comment.
Since it involves deletion of resource, we may want to be conservative. Perhaps would it be better to return an error if a version string is empty?
| @@ -112,3 +112,23 @@ func mustOpen(f string) *os.File { | |||
|
|
|||
| return r | |||
| } | |||
| func Remove(params Params) error { | |||
There was a problem hiding this comment.
Can you unit-test this code as well?
There was a problem hiding this comment.
To write unit cases for this function we need to refactor restapi package.as this function has dependencies with functions in restapi Package.
RaviKhandavilli
force-pushed
the
removecode
branch
from
feea5a7
to
8b476a9
Compare
RaviKhandavilli
force-pushed
the
removecode
branch
from
8b476a9
to
e3145c5
Compare
Added code to flamectl cli to Support remove command.
Added code to process remove command Http request on Api server and controller.
Added code to update DB , when a code is removed.