Skip to content

Conversation

@ishanarya0
Copy link
Member

@ishanarya0 ishanarya0 commented May 26, 2023

Related to raystack#449

@ishanarya0 ishanarya0 marked this pull request as draft May 26, 2023 10:13
@ishanarya0 ishanarya0 marked this pull request as ready for review May 26, 2023 11:55
@ishanarya0 ishanarya0 requested review from haveiss and sudo-suhas May 26, 2023 11:55
@sudo-suhas sudo-suhas changed the title feat: add support for retries on ext error feat: add support for retries on extractor error May 29, 2023
Copy link

@sudo-suhas sudo-suhas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, only minor change requested.

@sudo-suhas
Copy link

Was checking retries in caramlstore and merlin with this branch and there is a bug in caramlstore. Please add the following changes to the PR:

diff --git a/plugins/extractors/caramlstore/caramlstore.go b/plugins/extractors/caramlstore/caramlstore.go
index 8e1259b..3f16078 100644
--- a/plugins/extractors/caramlstore/caramlstore.go
+++ b/plugins/extractors/caramlstore/caramlstore.go
@@ -88,8 +88,6 @@ func (e *Extractor) Init(ctx context.Context, config plugins.Config) error {
 
 // Extract checks if the table is valid and extracts the table schema
 func (e *Extractor) Extract(ctx context.Context, emit plugins.Emit) error {
-	defer e.client.Close()
-
 	projects, err := e.client.Projects(ctx)
 	if err != nil {
 		if shouldRetry(err) {
@@ -136,6 +134,10 @@ func (e *Extractor) Extract(ctx context.Context, emit plugins.Emit) error {
 	return nil
 }
 
+func (e *Extractor) Close() error {
+	return e.client.Close()
+}
+
 func shouldRetry(err error) bool {
 	switch utils.StatusCode(err) {
 	case codes.Canceled,

ishanarya0 and others added 2 commits May 30, 2023 14:12
Co-authored-by: Suhas Karanth <sudo-suhas@users.noreply.github.com>
Copy link

@sudo-suhas sudo-suhas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ishanarya0 ishanarya0 removed the request for review from haveiss May 30, 2023 09:46
@ishanarya0 ishanarya0 merged commit 216669d into main May 30, 2023
@sudo-suhas sudo-suhas deleted the extractor-retries branch August 30, 2023 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants