You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by "klesh (via GitHub)" <gi...@apache.org> on 2023/02/23 10:22:14 UTC

[GitHub] [incubator-devlake] klesh opened a new pull request, #4507: fix: remove createdDateAfter completely

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

   ### Summary
   According to the Plan of  Attack from #4403, this is the last step of the BE side to replace `createdDateAfter` with `timeAfter`
   
   @mintsweet  the `config-ui` may have to be modified accordingly. you may close #4403 from your PR, thanks.
   
   ### Does this close any open issues?
   Part of #4403
   


-- 
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] likyh merged pull request #4507: fix: remove createdDateAfter completely

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


-- 
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 #4507: fix: remove createdDateAfter completely

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


##########
backend/helpers/pluginhelper/api/api_collector_with_state.go:
##########
@@ -66,37 +64,17 @@ func NewApiCollectorWithStateEx(args RawDataSubTaskArgs, createdDateAfter *time.
 	return &ApiCollectorStateManager{
 		RawDataSubTaskArgs: args,
 		LatestState:        latestState,
-		// Deprecating(timeAfter): to be deleted
-		CreatedDateAfter: createdDateAfter,
-		TimeAfter:        timeAfter,
-		ExecuteStart:     time.Now(),
+		TimeAfter:          timeAfter,
+		ExecuteStart:       time.Now(),
 	}, nil
 }
 
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-// Deprecating(timeAfter): use NewStatefulApiCollector instead
-func NewApiCollectorWithState(args RawDataSubTaskArgs, createdDateAfter *time.Time) (*ApiCollectorStateManager, errors.Error) {
-	return NewApiCollectorWithStateEx(args, createdDateAfter, nil)
-}
-
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-func NewStatefulApiCollector(args RawDataSubTaskArgs, timeAfter *time.Time) (*ApiCollectorStateManager, errors.Error) {
-	return NewApiCollectorWithStateEx(args, nil, timeAfter)
-}
-
 // IsIncremental indicates if the collector should operate in incremental mode
 func (m *ApiCollectorStateManager) IsIncremental() bool {
-	// the initial collection
-	if m.LatestState.LatestSuccessStart == nil {
-		return false
-	}
-	// prioritize TimeAfter parameter: collector should filter data by `updated_date`
 	if m.TimeAfter != nil {
 		return m.LatestState.TimeAfter == nil || !m.TimeAfter.Before(*m.LatestState.TimeAfter)
 	}
-	// Deprecating(timeAfter): to be removed
-	// fallback to CreatedDateAfter: collector should filter data by `created_date`
-	return m.LatestState.CreatedDateAfter == nil || m.CreatedDateAfter != nil && !m.CreatedDateAfter.Before(*m.LatestState.CreatedDateAfter)
+	return m.LatestState.LatestSuccessStart != nil

Review Comment:
   You are correct, thanks for pointing our. fixed



##########
backend/helpers/pluginhelper/api/api_collector_with_state.go:
##########
@@ -66,37 +64,17 @@ func NewApiCollectorWithStateEx(args RawDataSubTaskArgs, createdDateAfter *time.
 	return &ApiCollectorStateManager{
 		RawDataSubTaskArgs: args,
 		LatestState:        latestState,
-		// Deprecating(timeAfter): to be deleted
-		CreatedDateAfter: createdDateAfter,
-		TimeAfter:        timeAfter,
-		ExecuteStart:     time.Now(),
+		TimeAfter:          timeAfter,
+		ExecuteStart:       time.Now(),
 	}, nil
 }
 
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-// Deprecating(timeAfter): use NewStatefulApiCollector instead
-func NewApiCollectorWithState(args RawDataSubTaskArgs, createdDateAfter *time.Time) (*ApiCollectorStateManager, errors.Error) {
-	return NewApiCollectorWithStateEx(args, createdDateAfter, nil)
-}
-
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-func NewStatefulApiCollector(args RawDataSubTaskArgs, timeAfter *time.Time) (*ApiCollectorStateManager, errors.Error) {
-	return NewApiCollectorWithStateEx(args, nil, timeAfter)
-}
-
 // IsIncremental indicates if the collector should operate in incremental mode
 func (m *ApiCollectorStateManager) IsIncremental() bool {
-	// the initial collection
-	if m.LatestState.LatestSuccessStart == nil {
-		return false
-	}
-	// prioritize TimeAfter parameter: collector should filter data by `updated_date`
 	if m.TimeAfter != nil {
 		return m.LatestState.TimeAfter == nil || !m.TimeAfter.Before(*m.LatestState.TimeAfter)
 	}
-	// Deprecating(timeAfter): to be removed
-	// fallback to CreatedDateAfter: collector should filter data by `created_date`
-	return m.LatestState.CreatedDateAfter == nil || m.CreatedDateAfter != nil && !m.CreatedDateAfter.Before(*m.LatestState.CreatedDateAfter)
+	return m.LatestState.LatestSuccessStart != nil

Review Comment:
   You are correct, thanks for pointing out. fixed



-- 
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] likyh commented on a diff in pull request #4507: fix: remove createdDateAfter completely

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


##########
backend/helpers/pluginhelper/api/api_collector_with_state.go:
##########
@@ -66,37 +64,17 @@ func NewApiCollectorWithStateEx(args RawDataSubTaskArgs, createdDateAfter *time.
 	return &ApiCollectorStateManager{
 		RawDataSubTaskArgs: args,
 		LatestState:        latestState,
-		// Deprecating(timeAfter): to be deleted
-		CreatedDateAfter: createdDateAfter,
-		TimeAfter:        timeAfter,
-		ExecuteStart:     time.Now(),
+		TimeAfter:          timeAfter,
+		ExecuteStart:       time.Now(),
 	}, nil
 }
 
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-// Deprecating(timeAfter): use NewStatefulApiCollector instead
-func NewApiCollectorWithState(args RawDataSubTaskArgs, createdDateAfter *time.Time) (*ApiCollectorStateManager, errors.Error) {
-	return NewApiCollectorWithStateEx(args, createdDateAfter, nil)
-}
-
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-func NewStatefulApiCollector(args RawDataSubTaskArgs, timeAfter *time.Time) (*ApiCollectorStateManager, errors.Error) {
-	return NewApiCollectorWithStateEx(args, nil, timeAfter)
-}
-
 // IsIncremental indicates if the collector should operate in incremental mode
 func (m *ApiCollectorStateManager) IsIncremental() bool {
-	// the initial collection
-	if m.LatestState.LatestSuccessStart == nil {
-		return false
-	}
-	// prioritize TimeAfter parameter: collector should filter data by `updated_date`
 	if m.TimeAfter != nil {
 		return m.LatestState.TimeAfter == nil || !m.TimeAfter.Before(*m.LatestState.TimeAfter)
 	}
-	// Deprecating(timeAfter): to be removed
-	// fallback to CreatedDateAfter: collector should filter data by `created_date`
-	return m.LatestState.CreatedDateAfter == nil || m.CreatedDateAfter != nil && !m.CreatedDateAfter.Before(*m.LatestState.CreatedDateAfter)
+	return m.LatestState.LatestSuccessStart != nil

Review Comment:
   no



-- 
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] likyh commented on a diff in pull request #4507: fix: remove createdDateAfter completely

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


##########
backend/helpers/pluginhelper/api/api_collector_with_state.go:
##########
@@ -66,37 +64,17 @@ func NewApiCollectorWithStateEx(args RawDataSubTaskArgs, createdDateAfter *time.
 	return &ApiCollectorStateManager{
 		RawDataSubTaskArgs: args,
 		LatestState:        latestState,
-		// Deprecating(timeAfter): to be deleted
-		CreatedDateAfter: createdDateAfter,
-		TimeAfter:        timeAfter,
-		ExecuteStart:     time.Now(),
+		TimeAfter:          timeAfter,
+		ExecuteStart:       time.Now(),
 	}, nil
 }
 
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-// Deprecating(timeAfter): use NewStatefulApiCollector instead
-func NewApiCollectorWithState(args RawDataSubTaskArgs, createdDateAfter *time.Time) (*ApiCollectorStateManager, errors.Error) {
-	return NewApiCollectorWithStateEx(args, createdDateAfter, nil)
-}
-
-// NewApiCollectorWithState create a new ApiCollectorStateManager
-func NewStatefulApiCollector(args RawDataSubTaskArgs, timeAfter *time.Time) (*ApiCollectorStateManager, errors.Error) {
-	return NewApiCollectorWithStateEx(args, nil, timeAfter)
-}
-
 // IsIncremental indicates if the collector should operate in incremental mode
 func (m *ApiCollectorStateManager) IsIncremental() bool {
-	// the initial collection
-	if m.LatestState.LatestSuccessStart == nil {
-		return false
-	}
-	// prioritize TimeAfter parameter: collector should filter data by `updated_date`
 	if m.TimeAfter != nil {
 		return m.LatestState.TimeAfter == nil || !m.TimeAfter.Before(*m.LatestState.TimeAfter)
 	}
-	// Deprecating(timeAfter): to be removed
-	// fallback to CreatedDateAfter: collector should filter data by `created_date`
-	return m.LatestState.CreatedDateAfter == nil || m.CreatedDateAfter != nil && !m.CreatedDateAfter.Before(*m.LatestState.CreatedDateAfter)
+	return m.LatestState.LatestSuccessStart != nil

Review Comment:
   check LatestSuccessStart first. Because when LatestState exists but the latest collection fails (LatestSuccessStart=nil), it will be true that `m.TimeAfter != nil && m.LatestState.TimeAfter == nil`.



-- 
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