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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to