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

API cleanup #228

Open
yotamN opened this issue Jan 17, 2023 · 2 comments
Open

API cleanup #228

yotamN opened this issue Jan 17, 2023 · 2 comments

Comments

@yotamN
Copy link
Member

yotamN commented Jan 17, 2023

The current API is a bit messy, especially compared to comparative wrappers like frida-node. I think we have three main areas where we should focus our cleanup one:

  1. Core and init merge - if there is a reason for the split I would love to know but as it seems, the split only make the API confusing and verbose.
  2. Uniform interface - we have the C wrapper of Frida in src/_frida.c and the Python wrapper of the C wrapper at frida/core.py. I think we should try to merge them when we can, sometimes we have Python classes that are just dumb wrappers. When we can't merge them, we should only expose the Python wrapper no matter what, otherwise it become confusing really fast.
  3. Async everything - We only support Python 3.7+ and soon will drop support for 3.7 (EOF in June), so we can use the "new" asyncio package. We would keep the sync methods with a _sync prefix, at least I think we should.

The big question of course is how to handle this cleanup, it will break all usages of the library but it's probably for the best.

@oleavr
Copy link
Member

oleavr commented Jan 18, 2023

The current API is a bit messy, especially compared to comparative wrappers like frida-node. I think we have three main areas where we should focus our cleanup one:

  1. Core and init merge - if there is a reason for the split I would love to know but as it seems, the split only make the API confusing and verbose.

Agreed.

  1. Uniform interface - we have the C wrapper of Frida in src/_frida.c and the Python wrapper of the C wrapper at frida/core.py. I think we should try to merge them when we can, sometimes we have Python classes that are just dumb wrappers. When we can't merge them, we should only expose the Python wrapper no matter what, otherwise it become confusing really fast.

Sounds good.

  1. Async everything - We only support Python 3.7+ and soon will drop support for 3.7 (EOF in June), so we can use the "new" asyncio package. We would keep the sync methods with a _sync prefix, at least I think we should.

Love it!

The big question of course is how to handle this cleanup, it will break all usages of the library but it's probably for the best.

The next opportunity to do that will be Frida 17. The breakage is unfortunate, but OTOH it will completely eliminate a whole class of threading issues that I'm sure affect a lot of existing applications. (The current constraints are not really documented and thus poorly understood — but application developers shouldn't need to worry about such things to begin with.)

Longer term we should probably consider having bindings be versioned separately. We will need a lot of CI/automation in place for that to become practical though.

@yotamN
Copy link
Member Author

yotamN commented Feb 4, 2023

Could we maybe use Cython instead of having part of the library written in C and part written in Python? I haven't got to use it really but it seems to be a mature solution with many real world examples like Numpy and Pandas.

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

No branches or pull requests

2 participants