Skip to content

Conversation

@abhishekkumams
Copy link
Contributor

@abhishekkumams abhishekkumams commented Nov 21, 2022

Why make this change?

What is this change?

  • because of using IEnumerable, the fields are not null, instead they're empty IEnumerable List, hence our cli used to think it's a new value from user which removes all the current parameters.
  • We updated the check with Enumerable.Any() along with null, to not change the parameters if nothing is provided from user.

How was this tested?

  • Unit Tests

Steps To Verify

Initializing and adding a stored procedure entity

dab init -c test.json --database-type mssql
dab add Sptest -c test.json --source s001.test --source.type "stored-procedure" --source.params "id:1,name:Abhi" --permissions "anonymous:read"

Output

{
            "type": "stored-procedure",
            "object": "s001.test",
            "parameters": {
                "param1": 123,
                "param2": "hello",
                "param3": true
            }
 }

updating the source name

dab add Sptest -c test.json --source dbo.sptest
Output

{
            "type": "stored-procedure",
            "object": "dbo.sptest",
            "parameters": {
                "param1": 123,
                "param2": "hello",
                "param3": true
            },
            "key-fields": []
 }

Co-authored-by: Shyam Sundar J  <shyamsundarj@microsoft.com>
@abhishekkumams abhishekkumams merged commit 684dfaa into main Nov 21, 2022
@abhishekkumams abhishekkumams deleted the dev/abhishekkuma/fix_cli_stored_procedure_update branch November 21, 2022 06:19
""param2"": ""hello"",
""param3"": true
},
""key-fields"": []
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: since its a stored procedure, why do we even bother to mention key-fields here?

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

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Issue while updating Stored Procedure using CLI

6 participants