You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by "keon94 (via GitHub)" <gi...@apache.org> on 2023/01/21 00:00:00 UTC

[GitHub] [incubator-devlake] keon94 opened a new pull request, #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap

keon94 opened a new pull request, #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248

   ### Summary
   Pagerduty collector and config will be redone to be independent of Singer spec due to licensing conflicts.
   
   ### Does this close any open issues?
   Closes #4220 
   
   ### Screenshots
   Include any relevant screenshots here.
   
   ### Other Information
   Any other information that is important to this PR.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] keon94 commented on pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#issuecomment-1460612134

   On hold for now until we get an update from #4575


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1149966016


##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,125 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,

Review Comment:
   Oh good catch, I was using this for testing, forgot to remove it lol



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] keon94 commented on pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#issuecomment-1488269862

   @klesh I've fixed the problem.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] klesh commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1142916644


##########
backend/plugins/pagerduty/api/scope.go:
##########
@@ -0,0 +1,212 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"github.com/mitchellh/mapstructure"
+	"net/http"
+	"strconv"
+)
+
+type apiService struct {
+	models.Service
+	TransformationRuleName string `json:"transformationRuleName,omitempty"`
+}
+
+type req struct {
+	Data []*models.Service `json:"data"`
+}
+
+// PutScope create or update pagerduty repo
+// @Summary create or update pagerduty repo
+// @Description Create or update pagerduty repo
+// @Tags plugins/pagerduty
+// @Accept application/json
+// @Param connectionId path int true "connection ID"
+// @Param scope body req true "json"
+// @Success 200  {object} []models.Service
+// @Failure 400  {object} shared.ApiBody "Bad Request"
+// @Failure 500  {object} shared.ApiBody "Internal Error"
+// @Router /plugins/pagerduty/connections/{connectionId}/scopes [PUT]
+func PutScope(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {

Review Comment:
   @warren830 recently finished the `scopeHelper` , you might want to take a look https://github.com/apache/incubator-devlake/blob/main/backend/plugins/github/api/scope.go



##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())

Review Comment:
   Are you sure the `since` is filtering on the `created_at` field? I ask because `since` from `github` is filtering on the `updated_at` field.  Can you add the document link as a comment? I can't find their document 🥲



##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())
+						}
+					} else {
+						query.Set("date_range", "all")
+					}
+					query.Set("sort_by", "created_at:asc")
+					query.Set("limit", fmt.Sprintf("%d", reqData.Pager.Size))
+					query.Set("offset", fmt.Sprintf("%d", reqData.Pager.Page))
+					query.Set("total", "true")
+					return query, nil
+				},
+				ResponseParser: func(res *http.Response) ([]json.RawMessage, errors.Error) {
+					rawResult := collectedIncidents{}
+					err := api.UnmarshalResponse(res, &rawResult)
+					return rawResult.Incidents, err
+				},
+			},
+			GetNextPageCustomData: func(prevReqData *api.RequestData, prevPageResponse *http.Response) (interface{}, errors.Error) {
+				// not sure this is even necessary because the framework seems to auto-detect when to stop querying for the next page

Review Comment:
   This is NOT necessary because `GetNextPageCustomData` is designed for API that enforces a sequential page fetching, e.g.  github graphql, to fetch page 2, we must pass the `nextPageToken` from page 1.
   
   If the API supports filtering on the `updated_at`, we don't need `NewStatefulApiCollectorForFinalizableEntity` at all.
   
   If `since` is targeting the `created_at` field, and judged by the `Query` function, you might choose either `Determined` or `Undetermined` strategy depending on whether the API returns the total number of pages.
   
   Note the records must be sorted by `created_at` in **Descending** order in conjunction with `Concurrency` option so the collector would stop correctly by examining the created_at field
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1149905399


##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())
+						}
+					} else {
+						query.Set("date_range", "all")
+					}
+					query.Set("sort_by", "created_at:asc")
+					query.Set("limit", fmt.Sprintf("%d", reqData.Pager.Size))
+					query.Set("offset", fmt.Sprintf("%d", reqData.Pager.Page))
+					query.Set("total", "true")
+					return query, nil
+				},
+				ResponseParser: func(res *http.Response) ([]json.RawMessage, errors.Error) {
+					rawResult := collectedIncidents{}
+					err := api.UnmarshalResponse(res, &rawResult)
+					return rawResult.Incidents, err
+				},
+			},
+			GetNextPageCustomData: func(prevReqData *api.RequestData, prevPageResponse *http.Response) (interface{}, errors.Error) {
+				// not sure this is even necessary because the framework seems to auto-detect when to stop querying for the next page

Review Comment:
   I've removed it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1149902270


##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())

Review Comment:
   Yeah, I found this out through testing and experimentation. Their docs are not clear on what date this specifically means, but it's going to be the creation date. When I updated a property on the incident through their website, this value remained static, suggesting it is indeed the creation date. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] klesh commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1149959198


##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())

Review Comment:
   Does the API return `total page` or `total records` ? if so , please implement the `GetTotalPages` func



##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,125 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,

Review Comment:
   Why `PageSize = 1`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1149901334


##########
backend/plugins/pagerduty/api/scope.go:
##########
@@ -0,0 +1,212 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"github.com/mitchellh/mapstructure"
+	"net/http"
+	"strconv"
+)
+
+type apiService struct {
+	models.Service
+	TransformationRuleName string `json:"transformationRuleName,omitempty"`
+}
+
+type req struct {
+	Data []*models.Service `json:"data"`
+}
+
+// PutScope create or update pagerduty repo
+// @Summary create or update pagerduty repo
+// @Description Create or update pagerduty repo
+// @Tags plugins/pagerduty
+// @Accept application/json
+// @Param connectionId path int true "connection ID"
+// @Param scope body req true "json"
+// @Success 200  {object} []models.Service
+// @Failure 400  {object} shared.ApiBody "Bad Request"
+// @Failure 500  {object} shared.ApiBody "Internal Error"
+// @Router /plugins/pagerduty/connections/{connectionId}/scopes [PUT]
+func PutScope(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {

Review Comment:
   Ok. I've refactored it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1151334718


##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())

Review Comment:
   Yes it does. Done.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1151335668


##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())
+						}
+					} else {
+						query.Set("date_range", "all")
+					}
+					query.Set("sort_by", "created_at:asc")
+					query.Set("limit", fmt.Sprintf("%d", reqData.Pager.Size))
+					query.Set("offset", fmt.Sprintf("%d", reqData.Pager.Page))
+					query.Set("total", "true")
+					return query, nil
+				},
+				ResponseParser: func(res *http.Response) ([]json.RawMessage, errors.Error) {
+					rawResult := collectedIncidents{}
+					err := api.UnmarshalResponse(res, &rawResult)
+					return rawResult.Incidents, err
+				},
+			},
+			GetNextPageCustomData: func(prevReqData *api.RequestData, prevPageResponse *http.Response) (interface{}, errors.Error) {
+				// not sure this is even necessary because the framework seems to auto-detect when to stop querying for the next page

Review Comment:
   Done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] klesh merged pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh merged PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] keon94 commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "keon94 (via GitHub)" <gi...@apache.org>.
keon94 commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1149905399


##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())
+						}
+					} else {
+						query.Set("date_range", "all")
+					}
+					query.Set("sort_by", "created_at:asc")
+					query.Set("limit", fmt.Sprintf("%d", reqData.Pager.Size))
+					query.Set("offset", fmt.Sprintf("%d", reqData.Pager.Page))
+					query.Set("total", "true")
+					return query, nil
+				},
+				ResponseParser: func(res *http.Response) ([]json.RawMessage, errors.Error) {
+					rawResult := collectedIncidents{}
+					err := api.UnmarshalResponse(res, &rawResult)
+					return rawResult.Incidents, err
+				},
+			},
+			GetNextPageCustomData: func(prevReqData *api.RequestData, prevPageResponse *http.Response) (interface{}, errors.Error) {
+				// not sure this is even necessary because the framework seems to auto-detect when to stop querying for the next page

Review Comment:
   I've removed it. Looks like GetTotalPages for determined strategy is commented out?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] klesh commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1149960500


##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())
+						}
+					} else {
+						query.Set("date_range", "all")
+					}
+					query.Set("sort_by", "created_at:asc")
+					query.Set("limit", fmt.Sprintf("%d", reqData.Pager.Size))
+					query.Set("offset", fmt.Sprintf("%d", reqData.Pager.Page))
+					query.Set("total", "true")
+					return query, nil
+				},
+				ResponseParser: func(res *http.Response) ([]json.RawMessage, errors.Error) {
+					rawResult := collectedIncidents{}
+					err := api.UnmarshalResponse(res, &rawResult)
+					return rawResult.Incidents, err
+				},
+			},
+			GetNextPageCustomData: func(prevReqData *api.RequestData, prevPageResponse *http.Response) (interface{}, errors.Error) {
+				// not sure this is even necessary because the framework seems to auto-detect when to stop querying for the next page

Review Comment:
   It was restored in https://github.com/apache/incubator-devlake/pull/4626, but is yet to be tested.
   I will update the PR today, so you may merge it first.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] klesh commented on a diff in pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#discussion_r1149957167


##########
backend/plugins/pagerduty/tasks/incidents_collector.go:
##########
@@ -18,37 +18,137 @@ limitations under the License.
 package tasks
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/apache/incubator-devlake/core/dal"
 	"github.com/apache/incubator-devlake/core/errors"
 	"github.com/apache/incubator-devlake/core/plugin"
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
-	"github.com/apache/incubator-devlake/helpers/pluginhelper/tap"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/pagerduty/models"
+	"net/http"
+	"net/url"
+	"reflect"
+	"time"
 )
 
 const RAW_INCIDENTS_TABLE = "pagerduty_incidents"
 
 var _ plugin.SubTaskEntryPoint = CollectIncidents
 
+type (
+	pagingInfo struct {
+		Limit  *int  `json:"limit"`
+		Offset *int  `json:"offset"`
+		Total  *int  `json:"total"`
+		More   *bool `json:"more"`
+	}
+	collectedIncidents struct {
+		pagingInfo
+		Incidents []json.RawMessage `json:"incidents"`
+	}
+
+	collectedIncident struct {
+		pagingInfo
+		Incident json.RawMessage `json:"incident"`
+	}
+	simplifiedRawIncident struct {
+		IncidentNumber int       `json:"incident_number"`
+		CreatedAt      time.Time `json:"created_at"`
+	}
+)
+
 func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*PagerDutyTaskData)
-	collector, err := tap.NewTapCollector(
-		&tap.CollectorArgs[tap.SingerTapStream]{
-			RawDataSubTaskArgs: helper.RawDataSubTaskArgs{
-				Ctx:   taskCtx,
-				Table: RAW_INCIDENTS_TABLE,
-				Params: models.PagerDutyParams{
-					Stream:       models.IncidentStream,
-					ConnectionId: data.Options.ConnectionId,
+	db := taskCtx.GetDal()
+	args := api.RawDataSubTaskArgs{
+		Ctx: taskCtx,
+		Params: PagerDutyParams{
+			ConnectionId: data.Options.ConnectionId,
+		},
+		Table: RAW_INCIDENTS_TABLE,
+	}
+	collector, err := api.NewStatefulApiCollectorForFinalizableEntity(api.FinalizableApiCollectorArgs{
+		RawDataSubTaskArgs: args,
+		ApiClient:          data.Client,
+		TimeAfter:          data.TimeAfter,
+		CollectNewRecordsByList: api.FinalizableApiCollectorListArgs{
+			PageSize: 1,
+			GetCreated: func(item json.RawMessage) (time.Time, errors.Error) {
+				incident := &simplifiedRawIncident{}
+				err := json.Unmarshal(item, incident)
+				if err != nil {
+					return time.Time{}, errors.BadInput.Wrap(err, "failed to unmarshal incident")
+				}
+				return incident.CreatedAt, nil
+			},
+			FinalizableApiCollectorCommonArgs: api.FinalizableApiCollectorCommonArgs{
+				UrlTemplate: "incidents",
+				Query: func(reqData *api.RequestData, createdAfter *time.Time) (url.Values, errors.Error) {
+					query := url.Values{}
+					if createdAfter != nil {
+						now := time.Now()
+						if now.Sub(*createdAfter).Seconds() > 180*24*time.Hour.Seconds() {
+							// beyond 6 months Pagerduty API will just return nothing, so need to query for 'all' instead
+							query.Set("date_range", "all")
+						} else {
+							query.Set("since", data.TimeAfter.String())

Review Comment:
   Great, well done, please add this information to the comment 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] klesh commented on pull request #4248: [feat-4220][PagerDuty plugin]: Rework Pagerduty to not use Singer tap and support Blueprint v200

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on PR #4248:
URL: https://github.com/apache/incubator-devlake/pull/4248#issuecomment-1488001187

   @keon94 The e2e test had failed


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org