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

[Enhancement]: Update Airtable Handler to be an API handler and use Python SDK #10091

Open
HahaBill opened this issue Oct 30, 2024 · 11 comments · May be fixed by #10108
Open

[Enhancement]: Update Airtable Handler to be an API handler and use Python SDK #10091

HahaBill opened this issue Oct 30, 2024 · 11 comments · May be fixed by #10108
Assignees
Labels
enhancement New feature or request

Comments

@HahaBill
Copy link
Contributor

Short description of current behavior

The current handler uses API endpoints to connect to Airtable. Using SDK is more preferable.

For more context: #9812 (comment)

Video or screenshots

No response

Expected behavior

No response

How to reproduce the error

No response

Anything else?

No response

@HahaBill HahaBill added the bug Something isn't working label Oct 30, 2024
@HahaBill
Copy link
Contributor Author

@ZoranPandovski Hi, I created the issue for updating the Airtable Handler.

@HahaBill HahaBill linked a pull request Nov 3, 2024 that will close this issue
9 tasks
@HahaBill
Copy link
Contributor Author

HahaBill commented Nov 3, 2024

Draft PR: #10108

@HahaBill
Copy link
Contributor Author

HahaBill commented Nov 6, 2024

@ZoranPandovski @MinuraPunchihewa Hi, for this issue, I am planning on not only modifying the handler to use the SDK but also transforming the Airtable handler from Database handler to an API (Application) handler because it seems more intuitive for me since it uses restful api calls.

However, before proceeding further, I just want to discuss with you whether you agree on this because I think it is a big change and I don't want to delete/redo existing functionalities that may have some intended purposes.

@HahaBill
Copy link
Contributor Author

HahaBill commented Nov 6, 2024

The pyairtable has formulas which are used to filter and query records. In the current implementation, we are retrieving all records, merge them into pandas.Dataframe and use duckdb to do SQL operations. If we want to use their formulas, then I think API (Application) handler is a better fit but it's more involved to implement.

Final decision

I will just make the handler to use the Python SDK (pyairtable) as intended for this issue and in the future we can think about whether it is worth to use the formulas.

@MinuraPunchihewa
Copy link
Contributor

Hey @HahaBill,
Sorry for the late reply, I am in agreement with you that it is probably best to convert this to an API handler. If possible, go ahead and make this update.

@HahaBill
Copy link
Contributor Author

Hey @HahaBill, Sorry for the late reply, I am in agreement with you that it is probably best to convert this to an API handler. If possible, go ahead and make this update.

No worries! Alright, I will then convert the handler into API handler.

@HahaBill HahaBill changed the title [Enhancement]: Update Airtable Handler to use Python SDK instead of APIs [Enhancement]: Update Airtable Handler to be an API handler and use Python SDK Nov 17, 2024
@linear linear bot added enhancement New feature or request and removed bug Something isn't working labels Dec 5, 2024
@linear linear bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 5, 2024
@ZoranPandovski ZoranPandovski reopened this Dec 5, 2024
@HahaBill
Copy link
Contributor Author

HahaBill commented Dec 6, 2024

@ZoranPandovski @MinuraPunchihewa I've been struggling to make the SDK work in mindsdb, but finally, I have found a solution today: https://stackoverflow.com/questions/79234164/partially-initialized-module-pyairtable. It works!

This requires the Pydantic version to be less than 2.10. I saw that in requirements.txt in mindsdb that the Pydantic version has to be more or equal to 2.7.0. Is there an issue if we set the Pydantic version to be >=2.7.0 and <2.10?

@HahaBill
Copy link
Contributor Author

HahaBill commented Dec 6, 2024

But anyway, I am almost done. I only need more testing!

@MinuraPunchihewa
Copy link
Contributor

@HahaBill That's great!
Let's pin the version of pydantic to <2.10 in the requirements for the integration for now. Let's decide later if we want to update the main version as well.

@HahaBill
Copy link
Contributor Author

HahaBill commented Dec 6, 2024

@HahaBill That's great! Let's pin the version of pydantic to <2.10 in the requirements for the integration for now. Let's decide later if we want to update the main version as well.

Alright, got it!

@HahaBill
Copy link
Contributor Author

@ZoranPandovski @MinuraPunchihewa Ready for a review! PR: #10108

I saw that there had been some updates regarding the Pydantic version, it seems to work fine now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants