feat(restapi): remove base_url in restapi connector (#102)

Because - We should put the full URL in component setting, so that the user can have a better understanding of the meaning of the Restapi component. This commit - Move the URL field to the component configuration section, and only keep Authentication in the connector configuration.
instill-ai · Jan 12, 2024 · 34d1a20 · 34d1a20
1 parent 7613561
commit 34d1a20
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 63 deletions.
diff --git a/pkg/restapi/client.go b/pkg/restapi/client.go
@@ -7,8 +7,8 @@ import (
 )
 
 type TaskInput struct {
-	EndpointPath *string                `json:"endpoint_path,omitempty"`
-	Body         map[string]interface{} `json:"body,omitempty"`
+	EndpointUrl string                 `json:"endpoint_url"`
+	Body        map[string]interface{} `json:"body,omitempty"`
 }
 
 type TaskOutput struct {
@@ -18,7 +18,7 @@ type TaskOutput struct {
 }
 
 func newClient(config *structpb.Struct, logger *zap.Logger) (*httpclient.Client, error) {
-	c := httpclient.New("REST API", getBaseURL(config),
+	c := httpclient.New("REST API", "",
 		httpclient.WithLogger(logger),
 	)
 

diff --git a/pkg/restapi/config/definitions.json b/pkg/restapi/config/definitions.json
@@ -145,13 +145,6 @@
             "order": 1,
             "title": "Authentication",
             "type": "object"
-          },
-          "base_url": {
-            "description": "Base URL of the REST API",
-            "instillUIOrder": 0,
-            "order": 0,
-            "title": "Base URL",
-            "type": "string"
           }
         },
         "required": [

diff --git a/pkg/restapi/config/tasks.json b/pkg/restapi/config/tasks.json
@@ -3,7 +3,7 @@
     "inputWithBody": {
       "instillEditOnNodeFields": [
         "body",
-        "endpoint_path"
+        "endpoint_url"
       ],
       "instillUIOrder": 0,
       "properties": {
@@ -22,8 +22,8 @@
           "title": "Body",
           "type": "object"
         },
-        "endpoint_path": {
-          "description": "The API endpoint path relative to the base URL",
+        "endpoint_url": {
+          "description": "The API endpoint url",
           "instillAcceptFormats": [
             "string"
           ],
@@ -33,7 +33,7 @@
             "reference",
             "template"
           ],
-          "title": "Endpoint Path",
+          "title": "Endpoint Url",
           "type": "string"
         },
         "output_body_schema": {
@@ -56,16 +56,16 @@
         }
       },
       "required": [
-        "endpoint_path"
+        "endpoint_url"
       ],
       "title": "Input",
       "type": "object"
     },
     "inputWithoutBody": {
       "instillUIOrder": 0,
       "properties": {
-        "endpoint_path": {
-          "description": "The API endpoint path relative to the base URL",
+        "endpoint_url": {
+          "description": "The API endpoint url",
           "instillAcceptFormats": [
             "string"
           ],
@@ -75,7 +75,7 @@
             "reference",
             "template"
           ],
-          "title": "Endpoint Path",
+          "title": "Endpoint Url",
           "type": "string"
         },
         "output_body_schema": {
@@ -98,7 +98,7 @@
         }
       },
       "required": [
-        "endpoint_path"
+        "endpoint_url"
       ],
       "title": "Input",
       "type": "object"

diff --git a/pkg/restapi/connector_test.go b/pkg/restapi/connector_test.go
@@ -34,18 +34,14 @@ func TestConnector_Execute(t *testing.T) {
 	logger := zap.NewNop()
 	connector := Init(logger)
 	defID := uuid.Must(uuid.NewV4())
-
-	req := TaskInput{
-		EndpointPath: &path,
-		Body: map[string]any{
-			"title": "Be the wheel",
-		},
+	reqBody := map[string]any{
+		"title": "Be the wheel",
 	}
 
 	c.Run("nok - unsupported task", func(c *qt.C) {
 		task := "FOOBAR"
 
-		exec, err := connector.CreateExecution(defID, task, cfg("", noAuthType), logger)
+		exec, err := connector.CreateExecution(defID, task, cfg(noAuthType), logger)
 		c.Assert(err, qt.IsNil)
 
 		pbIn := new(structpb.Struct)
@@ -69,7 +65,7 @@ func TestConnector_Execute(t *testing.T) {
 
 			body, err := io.ReadAll(r.Body)
 			c.Assert(err, qt.IsNil)
-			c.Check(body, qt.JSONEquals, req.Body)
+			c.Check(body, qt.JSONEquals, reqBody)
 
 			w.Header().Set("Content-Type", httpclient.MIMETypeJSON)
 			w.WriteHeader(http.StatusBadRequest)
@@ -79,10 +75,13 @@ func TestConnector_Execute(t *testing.T) {
 		srv := httptest.NewServer(h)
 		c.Cleanup(srv.Close)
 
-		exec, err := connector.CreateExecution(defID, taskPost, cfg(srv.URL, basicAuthType), logger)
+		exec, err := connector.CreateExecution(defID, taskPost, cfg(basicAuthType), logger)
 		c.Assert(err, qt.IsNil)
 
-		pbIn, err := base.ConvertToStructpb(req)
+		pbIn, err := base.ConvertToStructpb(TaskInput{
+			EndpointUrl: srv.URL + path,
+			Body:        reqBody,
+		})
 		c.Assert(err, qt.IsNil)
 
 		got, err := exec.Execute([]*structpb.Struct{pbIn})
@@ -106,10 +105,14 @@ func TestConnector_Execute(t *testing.T) {
 		srv := httptest.NewServer(h)
 		c.Cleanup(srv.Close)
 
-		exec, err := connector.CreateExecution(defID, taskPut, cfg(srv.URL, apiKeyType), logger)
+		exec, err := connector.CreateExecution(defID, taskPut, cfg(apiKeyType), logger)
 		c.Assert(err, qt.IsNil)
 
-		pbIn, err := base.ConvertToStructpb(req)
+		pbIn, err := base.ConvertToStructpb(TaskInput{
+			EndpointUrl: srv.URL + path,
+			Body:        reqBody,
+		})
+
 		c.Assert(err, qt.IsNil)
 
 		got, err := exec.Execute([]*structpb.Struct{pbIn})
@@ -134,10 +137,13 @@ func TestConnector_Execute(t *testing.T) {
 		srv := httptest.NewServer(h)
 		c.Cleanup(srv.Close)
 
-		exec, err := connector.CreateExecution(defID, taskGet, cfg(srv.URL, bearerTokenType), logger)
+		exec, err := connector.CreateExecution(defID, taskGet, cfg(bearerTokenType), logger)
 		c.Assert(err, qt.IsNil)
 
-		pbIn, err := base.ConvertToStructpb(req)
+		pbIn, err := base.ConvertToStructpb(TaskInput{
+			EndpointUrl: srv.URL + path,
+			Body:        reqBody,
+		})
 		c.Assert(err, qt.IsNil)
 
 		got, err := exec.Execute([]*structpb.Struct{pbIn})
@@ -156,12 +162,6 @@ func TestConnector_Test(t *testing.T) {
 	connector := Init(logger)
 	defID := uuid.Must(uuid.NewV4())
 
-	c.Run("nok", func(c *qt.C) {
-		got, err := connector.Test(defID, cfg("http://no-such.host", noAuthType), logger)
-		c.Check(err, qt.IsNotNil)
-		c.Check(got, qt.Equals, pipelinePB.Connector_STATE_ERROR)
-	})
-
 	c.Run("ok - connected (even with non-2xx status", func(c *qt.C) {
 		h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			c.Check(r.Method, qt.Equals, http.MethodGet)
@@ -174,7 +174,7 @@ func TestConnector_Test(t *testing.T) {
 		srv := httptest.NewServer(h)
 		c.Cleanup(srv.Close)
 
-		got, err := connector.Test(defID, cfg(srv.URL, noAuthType), logger)
+		got, err := connector.Test(defID, cfg(noAuthType), logger)
 		c.Check(err, qt.IsNil)
 		c.Check(got, qt.Equals, pipelinePB.Connector_STATE_CONNECTED)
 	})
@@ -202,11 +202,10 @@ var testAuth = map[authType]map[string]any{
 	},
 }
 
-func cfg(basePath string, atype authType) *structpb.Struct {
+func cfg(atype authType) *structpb.Struct {
 	auth := testAuth[atype]
 	auth["auth_type"] = string(atype)
 	config, _ := structpb.NewStruct(map[string]any{
-		"base_url":       basePath,
 		"authentication": auth,
 	})
 

diff --git a/pkg/restapi/main.go b/pkg/restapi/main.go
@@ -78,10 +78,6 @@ func (c *Connector) CreateExecution(defUID uuid.UUID, task string, config *struc
 	return e, nil
 }
 
-func getBaseURL(config *structpb.Struct) string {
-	return config.GetFields()["base_url"].GetStringValue()
-}
-
 func getAuthentication(config *structpb.Struct) (authentication, error) {
 	auth := config.GetFields()["authentication"].GetStructValue()
 	authType := auth.GetFields()["auth_type"].GetStringValue()
@@ -121,10 +117,6 @@ func getAuthentication(config *structpb.Struct) (authentication, error) {
 }
 
 func (e *Execution) Execute(inputs []*structpb.Struct) ([]*structpb.Struct, error) {
-	client, err := newClient(e.Config, e.Logger)
-	if err != nil {
-		return nil, err
-	}
 
 	method, ok := taskMethod[e.Task]
 	if !ok {
@@ -138,14 +130,15 @@ func (e *Execution) Execute(inputs []*structpb.Struct) ([]*structpb.Struct, erro
 	for _, input := range inputs {
 		taskIn := TaskInput{}
 		taskOut := TaskOutput{}
-		path := ""
 
 		if err := base.ConvertFromStructpb(input, &taskIn); err != nil {
 			return nil, err
 		}
 
-		if taskIn.EndpointPath != nil {
-			path = *taskIn.EndpointPath
+		// We may have different url in batch.
+		client, err := newClient(e.Config, e.Logger)
+		if err != nil {
+			return nil, err
 		}
 
 		// An API error is a valid output in this connector.
@@ -154,7 +147,7 @@ func (e *Execution) Execute(inputs []*structpb.Struct) ([]*structpb.Struct, erro
 			req.SetBody(taskIn.Body)
 		}
 
-		resp, err := req.Execute(method, path)
+		resp, err := req.Execute(method, taskIn.EndpointUrl)
 		if err != nil {
 			return nil, err
 		}
@@ -173,15 +166,7 @@ func (e *Execution) Execute(inputs []*structpb.Struct) ([]*structpb.Struct, erro
 }
 
 func (c *Connector) Test(defUid uuid.UUID, config *structpb.Struct, logger *zap.Logger) (pipelinePB.Connector_State, error) {
-	client, err := newClient(config, logger)
-	if err != nil {
-		return pipelinePB.Connector_STATE_ERROR, err
-	}
-
-	if _, err := client.R().Get(""); err != nil {
-		return pipelinePB.Connector_STATE_ERROR, err
-	}
-
+	// we don't need to validate the connection since no url setting here
 	return pipelinePB.Connector_STATE_CONNECTED, nil
 }