-
Couldn't load subscription status.
- Fork 135
WIP: API Connector Refactor #1481
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
base: main
Are you sure you want to change the base?
Conversation
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.table = None | ||
|
|
||
| # | ||
| if inspect.isgenerator(lst): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's very convenient to write pagination code as a generator as i've done in this PR, and imo parsons should support that. #1529
| URI = "https://api.securevan.com/v4/" | ||
| SOAP_URI = "https://api.securevan.com/Services/V3/ListService.asmx?WSDL" | ||
|
|
||
| class VANConnector(APIConnector): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the van connector class is the one that knows about pagination and data unboxing.
| session = Scraper() | ||
| session.auth = ("default", api_key + "|" + str(self.db_code)) | ||
|
|
||
| self.connection = VANConnector(session=session, uri="https://api.securevan.com/v4/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with this pattern we don't need to keep a handle of the api key in this class.
| " MyVoters, MyCampaign, MyMembers, EveryAction." | ||
| ) | ||
|
|
||
| session = Scraper() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i like the idea of late binding the sessions object, but i'm not sure this the right pattern.
|
this is ready for an initial look and discussion @shaunagm @KasiaHinkson @sharinef1 |
|
Great, thanks @fgregg. @KasiaHinkson and @sharinef1 - maybe we could all try to take at least a quick look before the contributor meeting Thursday, so we can talk about it then? |
This is an experimental refactor of the API Connector class, applied to the NGP Van code.
It adds
through using the scraplib's scraper as a drop in replacement for the request session object.
this makes the API connector class much simpler. really just handling URI building and setting up headers. pagination and data unrolling are moved to the van connector class.
despite this PR not being marked as a draft, it should not be merged until more discussion. it is not in draft mode right now in order to allow for comments.
relates to #1456 and #736