Added trigger sync and save #230

eddbbt · 2023-10-16T13:17:40Z

Add command to sync local trigger to remote astarte cluster, after validation
Add command to save remote astarte trigger to local json files

Depends on astarte-platform/astarte-go#59

Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com> Signed-off-by: Eddy Babetto <eddy.babetto@gmail.com>

cmd/realm/triggers.go

Refactor of functions with pre-existing ones in order to save lines of code Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>

Add command triggers validate for validate json payload Signed-off-by: Eddy Babetto <eddy.babetto@gmail.com>

Annopaolo · 2023-11-15T13:38:14Z

cmd/realm/triggers.go

@@ -69,111 +71,372 @@ var triggersDeleteCmd = &cobra.Command{
 	Aliases: []string{"del"},
 }

+var triggersSaveCmd = &cobra.Command{
+	Use:   "save [destination-path]",


Let's keep consistency with other commands

Suggested change

Use: "save [destination-path]",

Use: "save <destination-path>",

Annopaolo · 2023-11-15T13:38:44Z

cmd/realm/triggers.go

+	Use:   "save [destination-path]",
+	Short: "Save triggers to a local folder",
+	Long: `Save each trigger in a realm to a local folder. Each trigger will
+be saved in a dedicated file whose name will be in the form '<trigger_name>_v<version>.json'.


Triggers are not versioned

Suggested change

be saved in a dedicated file whose name will be in the form '<trigger_name>_v<version>.json'.

be saved in a dedicated file whose name will be in the form '<trigger_name>.json'.

Annopaolo · 2023-11-15T13:40:06Z

cmd/realm/triggers.go

+}
+
+var triggersSyncCmd = &cobra.Command{
+	Use:   "sync <interface_files> [...]",


Suggested change

Use: "sync <interface_files> [...]",

Use: "sync <trigger_files> [...]",

Annopaolo · 2023-11-15T13:43:21Z

cmd/realm/triggers.go

+}
+
+func triggersShowF(command *cobra.Command, args []string) error {
+


Remove newline

cmd/realm/triggers.go

Annopaolo · 2023-11-15T14:05:47Z

cmd/realm/triggers.go

+
+		fmt.Printf("\n")
+		fmt.Printf("The following new triggers will be installed: %+q \n", list)
+		fmt.Printf("\n")


There's a \n at the end of the above print, no need for another

Annopaolo · 2023-11-15T14:06:59Z

cmd/realm/triggers.go

+			return err
+		}
+
+		var astarteTrigger map[string]interface{}


Note: once astarte-go offers utilities for triggers, this will be something like map[string]triggers.Trigger{}

Annopaolo · 2023-11-15T14:08:27Z

cmd/realm/triggers.go

-	triggerDefinition, _ := getTriggerRes.Parse()
-	respJSON, _ := json.MarshalIndent(triggerDefinition, "", "  ")
-	fmt.Println(string(respJSON))
+	triggerToInstall := []map[string]interface{}{}


Suggested change

triggerToInstall := []map[string]interface{}{}

triggersToInstall := []map[string]interface{}{}

Annopaolo · 2023-11-15T14:08:38Z

cmd/realm/triggers.go

-	respJSON, _ := json.MarshalIndent(triggerDefinition, "", "  ")
-	fmt.Println(string(respJSON))
+	triggerToInstall := []map[string]interface{}{}
+	triggerToUpdate := []map[string]interface{}{}


Suggested change

triggerToUpdate := []map[string]interface{}{}

triggersToUpdate := []map[string]interface{}{}

Annopaolo · 2023-11-15T14:08:55Z

cmd/realm/triggers.go

-	fmt.Println(string(respJSON))
+	triggerToInstall := []map[string]interface{}{}
+	triggerToUpdate := []map[string]interface{}{}
+	invalidtrigger := []string{}


Suggested change

invalidtrigger := []string{}

invalidTriggers := []string{}

Annopaolo · 2023-11-17T09:57:30Z

cmd/realm/triggers.go


+	RealmManagementCmd.AddCommand(triggersCmd)
+	triggersSyncCmd.Flags().Lookup("force").NoOptDefVal = "false"


I think that the flag should have NoOptDefVal set to true, so that one could write astartectl realm-management triggers sync ... --force (note the missing =true) and force the trigger synchronization

Also, if you set the NoOptDefVal here rather than in the sync function, I think that the lookup is redundant: something like this could work, too:

tiggersSyncCmd.PersistentFlags().BoolVar(...) tiggersSyncCmd.Flag("force").NoOptDefVal = "true"

Annopaolo · 2023-11-17T10:07:50Z

cmd/realm/triggers.go

+		}
+
+		// var astarteTrigger map[string]triggers.Trigger{}
+		var astarteTrigger map[string]interface{}


I think this should be a triggers.AstarteTrigger{} as your above comment suggests

Annopaolo · 2023-11-17T10:08:37Z

cmd/realm/triggers.go

+			return err
+		}
+
+		if _, err := getTriggerDefinition(realm, astarteTrigger["name"].(string)); err != nil {


Suggested change

if _, err := getTriggerDefinition(realm, astarteTrigger["name"].(string)); err != nil {

if _, err := getTriggerDefinition(realm, astarteTrigger.name); err != nil {

Annopaolo · 2023-11-17T10:08:55Z

cmd/realm/triggers.go

+
+		fmt.Printf("\n")
+		fmt.Printf("The following new triggers will be installed: %+q \n", list)
+		fmt.Printf("\n")


Annopaolo · 2023-11-17T10:10:12Z

cmd/realm/triggers.go

+			return nil
+		}
+
+		for _, v := range triggersToInstall {


Since one of the fileds is used in the for body, give a meaningful name to v (e.g. trigger)

Annopaolo · 2023-11-17T10:13:59Z

cmd/realm/triggers.go

+	return nil
+}
+
+func updateTrigger(realm string, triggername string, newtrig map[string]interface{}) error {


Suggested change

func updateTrigger(realm string, triggername string, newtrig map[string]interface{}) error {

func updateTrigger(realm string, triggerName string, newTrigger triggers.AstarteTrigger) error {

Annopaolo · 2023-11-17T10:15:08Z

cmd/realm/triggers.go


-	_, _ = deleteTriggerRes.Parse()
+func getTriggerDefinition(realm, triggerName string) (map[string]interface{}, error) {


Suggested change

func getTriggerDefinition(realm, triggerName string) (map[string]interface{}, error) {

func getTriggerDefinition(realm, triggerName string) (triggers.AstarteTrigger, error) {

Annopaolo · 2023-11-17T10:15:56Z

cmd/realm/triggers.go

+	for _, v := range triggersToInstall {
+		list = append(list, v["name"].(string))
+	}


Suggested change

for _, v := range triggersToInstall {

list = append(list, v["name"].(string))

}

for _, trigger := range triggersToInstall {

list = append(list, trigger.name)

}

Annopaolo · 2023-11-17T10:17:51Z

cmd/realm/triggers.go

+	triggersToInstall := []map[string]interface{}{}
+	triggersToUpdate := []map[string]interface{}{}
+	invalidTriggers := []string{}


Suggested change

triggersToInstall := []map[string]interface{}{}

triggersToUpdate := []map[string]interface{}{}

invalidTriggers := []string{}

triggersToInstall := []triggers.AstarteTrigger{}

triggersToUpdate := []triggers.AstarteTrigger{}

invalidTriggers := []string{}

Annopaolo · 2023-11-17T10:18:22Z

cmd/realm/triggers.go

+	for _, v := range triggersToUpdate {
+		list_existing = append(list_existing, v["name"].(string))
+	}


See above comment

Annopaolo

Some nitpicking, but the PR is good!

Annopaolo · 2023-11-20T11:50:41Z

.gitignore

@@ -5,6 +5,7 @@ astartectl_*
 .gobuild
 dist/
 /result
+.idea/


If this line is used, put this change in a separate commit

Annopaolo · 2023-11-20T14:43:42Z

go.sum

 github.com/Azure/go-autorest/autorest/adal v0.9.13/go.mod h1:W/MM4U6nLxnIskrw4UwWzlHfGjwUS50aOsc/I3yuU8M=
+github.com/Azure/go-autorest/autorest/adal v0.9.23 h1:Yepx8CvFxwNKpH6ja7RZ+sKX+DWYNldbLiALMC3BTz8=
+github.com/Azure/go-autorest/autorest/adal v0.9.23/go.mod h1:5pcMqFkdPhviJdlEy3kC/v1ZLnQl0MH6XA5YCcMhy4c=


Put dependency update in a different commit, too

Annopaolo · 2023-11-20T14:44:17Z

cmd/realm/triggers.go

+		list = append(list, trigger.Name)
+	}
+
+	list_existing := []string{}


Let's keep naming consistency

Suggested change

list_existing := []string{}

listExisting := []string{}

Add trigger validation on sync, removing invalid payload from process queue Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>

Edit .gitignore file to remove ide temp folder Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>

Update project dependencies to latest Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>

eddbbt force-pushed the master branch 5 times, most recently from df907e7 to 72a6616 Compare October 16, 2023 14:57

added trigger save and sync from and to json files

8a4814a

Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com> Signed-off-by: Eddy Babetto <eddy.babetto@gmail.com>

eddbbt force-pushed the master branch 2 times, most recently from ddb1c41 to 8a4814a Compare October 18, 2023 13:51

Annopaolo requested changes Nov 6, 2023

View reviewed changes

eddbbt force-pushed the master branch from 5b33405 to 7339059 Compare November 6, 2023 17:05

Refactor of install, list and show functions for triggers

a6f85c6

Refactor of functions with pre-existing ones in order to save lines of code Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>

eddbbt force-pushed the master branch from 7339059 to a6f85c6 Compare November 6, 2023 17:18

Add validation command

d72bb53

Add command triggers validate for validate json payload Signed-off-by: Eddy Babetto <eddy.babetto@gmail.com>

eddbbt force-pushed the master branch from d5d83c5 to c4af0bf Compare November 7, 2023 11:14

eddbbt mentioned this pull request Nov 7, 2023

Trigger sync upload #231

Closed

eddbbt force-pushed the master branch from c4af0bf to 8a982e6 Compare November 7, 2023 11:22

eddbbt requested a review from Annopaolo November 10, 2023 16:45

Annopaolo requested changes Nov 15, 2023

View reviewed changes

eddbbt force-pushed the master branch 3 times, most recently from 2f9c28b to 9197109 Compare November 16, 2023 13:42

eddbbt requested a review from Annopaolo November 16, 2023 13:58

eddbbt force-pushed the master branch 2 times, most recently from fec6e1a to da15c68 Compare November 17, 2023 09:36

Annopaolo requested changes Nov 17, 2023

View reviewed changes

eddbbt force-pushed the master branch 5 times, most recently from f1170a9 to b8d5f59 Compare November 17, 2023 17:14

eddbbt force-pushed the master branch 4 times, most recently from 1e359d0 to 9114d2a Compare November 20, 2023 12:00

Annopaolo reviewed Nov 20, 2023

View reviewed changes

eddbbt added 3 commits November 20, 2023 16:10

Add trigger validation

5b50d25

Add trigger validation on sync, removing invalid payload from process queue Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>

Edit .gitignore

f42648b

Edit .gitignore file to remove ide temp folder Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>

Update dependencies

7b065bf

Update project dependencies to latest Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>

eddbbt force-pushed the master branch from 9114d2a to 7b065bf Compare November 20, 2023 15:11

Annopaolo self-requested a review November 20, 2023 15:20

Annopaolo approved these changes Nov 20, 2023

View reviewed changes

Annopaolo merged commit 000f486 into astarte-platform:master Nov 20, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added trigger sync and save #230

Added trigger sync and save #230

eddbbt commented Oct 16, 2023 •

edited

Loading

Annopaolo Nov 15, 2023

Annopaolo Nov 15, 2023

Annopaolo Nov 15, 2023

Annopaolo Nov 15, 2023

Annopaolo Nov 15, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 15, 2023

Annopaolo Nov 15, 2023

Annopaolo Nov 15, 2023

Annopaolo Nov 15, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo Nov 17, 2023

Annopaolo left a comment

Annopaolo Nov 20, 2023

Annopaolo Nov 20, 2023

Annopaolo Nov 20, 2023

	Use: "save [destination-path]",
	Use: "save <destination-path>",

	be saved in a dedicated file whose name will be in the form '<trigger_name>_v<version>.json'.
	be saved in a dedicated file whose name will be in the form '<trigger_name>.json'.

	Use: "sync <interface_files> [...]",
	Use: "sync <trigger_files> [...]",

		}

		func triggersShowF(command *cobra.Command, args []string) error {

	triggerToInstall := []map[string]interface{}{}
	triggersToInstall := []map[string]interface{}{}

	triggerToUpdate := []map[string]interface{}{}
	triggersToUpdate := []map[string]interface{}{}


		RealmManagementCmd.AddCommand(triggersCmd)
		triggersSyncCmd.Flags().Lookup("force").NoOptDefVal = "false"

	if _, err := getTriggerDefinition(realm, astarteTrigger["name"].(string)); err != nil {
	if _, err := getTriggerDefinition(realm, astarteTrigger.name); err != nil {

	func updateTrigger(realm string, triggername string, newtrig map[string]interface{}) error {
	func updateTrigger(realm string, triggerName string, newTrigger triggers.AstarteTrigger) error {


		_, _ = deleteTriggerRes.Parse()
		func getTriggerDefinition(realm, triggerName string) (map[string]interface{}, error) {

Added trigger sync and save #230

Added trigger sync and save #230

Conversation

eddbbt commented Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Annopaolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddbbt commented Oct 16, 2023 •

edited

Loading