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

Download adapters and components #52

Open
wants to merge 53 commits into
base: develop
Choose a base branch
from

Conversation

CremaLuca
Copy link
Member

@CremaLuca CremaLuca commented Sep 11, 2021

Premise

The download phase was very much specific for each provider, leading to difficulty in spotting bugs, hard to test code, and multiple failure points.

Features

feat(Adapter): Class that holds components, performs the prepare, retrieve and atomize phases calling each one of its components per phase.
feat(AdapterComponent): Implements one or more of the three phases of the download, just like a middleware. They should be as generic as possible, making them harder to test but suitable for more providers.

Components

Parameter validation: Every downloader needs parameter validation because the provider only supports some values (eg. interval "1min" or "1m"), so it's a component that given the parameter name, and a validation method applies the method to check for validity of the parameter's value.

Request: Performs bare HTTP requests, taking values from parameters or constant ones given in the constructor. It can build a URL by using BASE_URL + url parameters + '?' + query parameters (not done manually, but by the requests module). Also supports HTTP headers, request timeout time, automatic JSON of the response, request limiter.

"Chunker": Splits an array of elements in an array of chunks of a given maximum size. eg max_size=3 then [A, B, C, D, E] - > [[A, B, C], [D, E]]. Used when the provider supports multiple tickers but sets a maximum number of them per request.

Notes

I also wanted to keep downloaders and the filtering/streaming structures separated, in order for the downloader part to be an MVP.

Picture of something weird

Oke

…sses

feat(ticker splitter): component that splits tickers in groups
test(ticker splitter)
fix(Parameter validator): items() instead of enumerate()
fix(Parameter validator): not more checking parameter is defined by default
fix(match param gen): now returns the generated method
test(Parameter validator)
… by adapter's parameters

style(adapters): formatted file
…ve for all sub-components

feat(Tradier): started adapter translation
feat(TickerSplitter): parametrised output ticker groups name
feat(Request): added debug mode
fix(Request): fixed status code, fixed json response
test(tradier_timeseries)
feat(TickerExtractor): added type and length checks
test(TickerExtractor)
feat(Pram valid.): added new datetime validation
test(param valid): added dt valid. tests
del(TradierTimeseries)
… reuse the same adapter for different downloads
@CremaLuca
Copy link
Member Author

Ayo bro let me review that for you

@CremaLuca
Copy link
Member Author

Ayo thanks bro

otri/downloader/tradier.py Outdated Show resolved Hide resolved
@CremaLuca
Copy link
Member Author

Still hard to read and decipher, needs some work.

Copy link
Contributor

@ricdezen ricdezen left a comment

Choose a reason for hiding this comment

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

Seems sort of fine overall, I only have two doubts I pointed out. Also I believe docstrings could be a bit more generous with explanations/examples. I don't think I would have understood what some of the components do if I could not have looked directly into the code.

@CremaLuca CremaLuca added the enhancement New feature or request label Sep 14, 2021
@CremaLuca CremaLuca self-assigned this Sep 14, 2021
Luca added 10 commits September 16, 2021 10:16
test: tradier checking provider and ticker in atoms
test: fixed some adapter component tests
fix: Tradier checking header presence before using it
…nt lists

delete: unused components, for clarity
rename: TickerChunkComp to ChunkerComp (more generic)
move: Parameter validation methods factory to validators file
refactor: SubAdapter does not make a difference between prep and ret components
test: updated components tests
@CremaLuca
Copy link
Member Author

Something that will need some work is how to pass a limiter to the request component at initialization, as of now the limiter is instantiated when the file is parsed and cannot be changed.

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 this pull request may close these issues.

2 participants