Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Refactor][Framework] Refactor Extractors and Converters to use Generics #3616

Closed
keon94 opened this issue Oct 29, 2022 · 13 comments
Closed
Assignees
Labels
good first issue Good for newcomers Stale type/refactor This issue is to refactor existing code
Milestone

Comments

@keon94
Copy link
Contributor

keon94 commented Oct 29, 2022

What and why to refactor

Extract functions use raw objects which they manually have to json-deserialize into predefined structs. This is needless boilerplate now that Go supports generics. Refactor the api_extractor.go (and other relevant framework files) to performs these deserializations under the hood and send the resulting Go structs to the user (Extract) functions.
The Converters suffer from this as well, although there's less boilerplate there. Refactor accordingly.

Describe the solution you'd like

Example:
currently we have:

Extract: func(row *helper.RawData) ([]interface{}, errors.Error) {
	apiAccountOrgs := &[]GithubAccountOrgsResponse{}
	err := json.Unmarshal(row.Data, apiAccountOrgs)
	if err != nil {
		return nil, errors.Convert(err)
	}
	simpleAccount := &SimpleAccountWithId{}
	err = json.Unmarshal(row.Input, simpleAccount)
	if err != nil {
		return nil, errors.Convert(err)
	}
	// more logic...
}

it should become like:

Extract: func(orgs []GithubAccountOrgsResponse, input *SimpleAccountWithId) ([]interface{}, errors.Error) {
	// more logic...
}

The API extractor will need to take two generic parameters: for the row data and the row input - initialization will look like this:

helper.NewApiExtractor[[]GithubAccountOrgsResponse, SimpleAccountWithId](...)

Related issues

n/a

Additional context

n/a

@keon94 keon94 added the type/refactor This issue is to refactor existing code label Oct 29, 2022
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity for 30 days. It will be closed in next 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Nov 29, 2022
@github-actions github-actions bot added the Stale label Dec 30, 2022
@klesh klesh removed the Stale label Jan 3, 2023
@klesh klesh added this to the v0.16.0 milestone Jan 3, 2023
@keon94 keon94 added the good first issue Good for newcomers label Jan 20, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity for 30 days. It will be closed in next 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Feb 20, 2023
@Startrekzky
Copy link
Contributor

Bump

@apache apache deleted a comment from github-actions bot Feb 20, 2023
@github-actions github-actions bot removed the Stale label Feb 21, 2023
@coldgust
Copy link
Member

coldgust commented Mar 6, 2023

Hello, I would like to finish this task,could you assign this to me?

@keon94
Copy link
Contributor Author

keon94 commented Mar 6, 2023

@coldgust Thanks for your interest. I've assigned this to you.

@likyh likyh pinned this issue Mar 7, 2023
@coldgust
Copy link
Member

coldgust commented Mar 9, 2023

It seems that most extractors just need to json-deserialize row.Data field, such as backend/plugins/github/tasks/account_extractor.go.

I want to define like this

type ApiExtractorArgs[T any] struct {
	RawDataSubTaskArgs
	Params    interface{}
	Extract   func(data *T, row *RawData) ([]interface{}, errors.Error)
	BatchSize int
}

It still pass *RawData parameter, and the user dicide to whether json-deserialize row.Input.

@keon94 keon94 removed this from the v0.16.0 milestone Apr 6, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity for 30 days. It will be closed in next 7 days if no further activity occurs.

@Dev79844
Copy link

Hey @keon94 ! Is anyone working on this issue? If not can you assign it to me

@keon94
Copy link
Contributor Author

keon94 commented Jun 20, 2023

@Dev79844 I don't believe so. Sure, you can have it. Thanks for taking it on.

@keon94 keon94 assigned Dev79844 and unassigned coldgust Jun 20, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity for 30 days. It will be closed in next 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Jul 26, 2023
@coldgust coldgust removed the Stale label Jul 26, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has been inactive for 60 days. It will be closed in next 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Sep 25, 2023
@klesh klesh removed the Stale label Sep 25, 2023
@klesh klesh added this to the v0.21 milestone Sep 25, 2023
Copy link

This issue has been automatically marked as stale because it has been inactive for 60 days. It will be closed in next 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Nov 25, 2023
Copy link

github-actions bot commented Dec 2, 2023

This issue has been closed because it has been inactive for a long time. You can reopen it if you encounter the similar problem in the future.

@github-actions github-actions bot closed this as completed Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Stale type/refactor This issue is to refactor existing code
Projects
None yet
Development

No branches or pull requests

7 participants