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

ported lbrynet.extras.daemon.analytics to asyncio #1745

Merged
merged 4 commits into from Jan 11, 2019

Conversation

osilkin98
Copy link
Contributor

PR Checklist

Please check all that apply to this PR using "x":

  • I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
  • I have checked that this PR does not introduce a breaking change

PR Type

What kind of change does this PR introduce?
This removes the use of treq and twisted modules from being used in analytics.py. It also removes the old LoopingCall mechanism and replaces its manager component with a dict that stores the looping Tasks as a name -> Task key value pair, which makes use of the looping_call function from lbrynet.extras.daemon.storage.py. Modification is also made to the looping_call function in order to catch the task cancellation error when being wrapped as a task coroutine.

Instead of treq, the aiohttp library is used, although instead of creating a ClientSession object, I use the aiohttp.request ContextManager to set the Api's cookies as that of the response's.

Why is this change necessary?
To remove the godawful dependency that is twisted

  • Bugfix
  • Feature
  • Breaking changes (bugfix or feature that introduces breaking changes)
  • Code style update (formatting)
  • Refactoring (no functional changes)
  • Documentation changes
  • Other - Please describe: Removes unwanted dependencies

@jackrobison
Copy link
Member

@osilkin98 can you rebase/squash this down to one or two commits on top of master?

@osilkin98 osilkin98 force-pushed the asyncio-analytics branch 2 times, most recently from eb20647 to 57375a5 Compare January 8, 2019 23:44
@eukreign
Copy link
Member

eukreign commented Jan 9, 2019

@osilkin98 please let me know when this is done and ready for review, i'll likely have some comments

@codecov
Copy link

codecov bot commented Jan 9, 2019

Codecov Report

Merging #1745 into master will decrease coverage by 0.01%.
The diff coverage is 41.02%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1745      +/-   ##
==========================================
- Coverage   63.84%   63.82%   -0.02%     
==========================================
  Files         150      150              
  Lines       13656    13641      -15     
==========================================
- Hits         8718     8707      -11     
+ Misses       4938     4934       -4
Impacted Files Coverage Δ
lbrynet/extras/daemon/DaemonConsole.py 37.43% <ø> (+0.19%) ⬆️
lbrynet/extras/daemon/Components.py 48.44% <0%> (ø) ⬆️
lbrynet/extras/daemon/analytics.py 58.1% <39.34%> (-0.54%) ⬇️
lbrynet/extras/daemon/Daemon.py 56.76% <50%> (ø) ⬆️
lbrynet/extras/looping_call_manager.py 62.5% <0%> (-12.5%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd7811f...647ed8d. Read the comment docs.

with `asyncio` and `aiohttp`. Also removes the calling loop from the
analytics in favor of a `dict` that stores the names of the methods
and their coroutines that are wrapped inside a looping task. The tasks
are canceled when the analytics manager is asked to shutdown

Signed-off-by: Oleg Silkin <o.silkin98@gmail.com>
Signed-off-by: Oleg Silkin <o.silkin98@gmail.com>
@eukreign eukreign merged commit 6ae4e68 into lbryio:master Jan 11, 2019
@eukreign eukreign changed the title Removes twisted dependency from lbrynet.extras.daemon.analytics.py ported lbrynet.extras.daemon.analytics to asyncio Jan 11, 2019
@eukreign eukreign added type: refactor Minimal user-visible changes, but significant internal work area: other labels Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: other type: refactor Minimal user-visible changes, but significant internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants