-
Notifications
You must be signed in to change notification settings - Fork 452
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
GuiDBTuples credit mining removal #2398
Conversation
Mind to review, @devos50, @lfdversluis? |
1b78780
to
a30122d
Compare
self.loaded_torrent[infohash] = tdef | ||
|
||
if callback is not None: | ||
callback(tdef) |
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.
you may want to use a deferred here
@lfdversluis pushed commits on your feedback. What do you think? |
6694c02
to
5f5db94
Compare
retest this please |
retest this please |
retest this please error in mac, TestDownloadsEndpoint.test_get_downloads |
self.torrents[infohash]['num_seeders'] = torrent.swarminfo[0] or 0 | ||
self.torrents[infohash]['num_leechers'] = torrent.swarminfo[1] or 0 | ||
self.torrents[infohash]['name'] = torrent.get_name() | ||
self.torrents[infohash]['metainfo'] = torrent |
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.
Does the torrent
object really represent the metainfo
of the torrent?
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 think yes, torrent
object is a TorrentDef
type
@ardhipoetra I gave my comments. Overall, it looks good and I'm glad you are able to remove much code in this PR :) |
@devos50 processed! :D |
retest this please credit mining error in Mac |
retest this please |
@ardhipoetra looks fine, let's put it on ready so @whirm can take a look. |
@devos50 Thanks! :D |
This check_and_register() function looks a bit fishy... why do you need this? Also the |
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.
@ardhipoetra looks good 👍 . Besides my looping call comment, I have one additional question: can you squash the first commit (where you remove the dependency on GUIDBTuple) and the commit(s) where you refactor the _load_torrent
method to use deferreds?
After that, please squash and I will merge if it's ok.
""" | ||
check if a torrent has been loaded | ||
""" | ||
if defer_param is None: |
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.
Please use the looping call mechanism here that I recently adopted in your tests (see the other tests in this file to see how I did that).
Closes Tribler#2356 Squashed : - Refactor to use deferred
1e2d71c
to
2094ebd
Compare
retest this please looks good |
retest this please last run okay |
retest this please last run : error in linux, get_settings Rest Api |
retest this please |
looks stable enough for me. What do you think @devos50? |
retest this please |
5 similar comments
retest this please |
retest this please |
retest this please |
retest this please |
retest this please |
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.
Please remove the indicated commented-out code line, squash your last two commits (where you create the test and refactor it with the looping call) and I will merge 👍
def add_to_loaded(infohash_str): | ||
""" | ||
function to add loaded infohash to memory | ||
""" | ||
self.loaded_torrent[unhexlify(infohash_str)].callback( | ||
TorrentDef.load_from_memory(self.session.get_collected_torrent(unhexlify(infohash_str)))) | ||
self.loaded_torrent[unhexlify(infohash_str)] = None | ||
|
||
# self.loaded_torrent[unhexlify(infohash_str)] = None |
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.
Please remove this commented-out code.
Squashed commits with : - Use looping call in checking loaded torrent
2094ebd
to
5b6a6f0
Compare
@devos50 ready! |
As fix for Issue #2356.
This will remove
Main
package dependencies on credit mining. It might be slow when downloading the credit source, but it works. How can I download the torrent via DHT/Peers?Btw, made PR into
next
because this is a bugfix for experimental feature (credit mining)