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

WIP-Collective Effort: Rework GUI to support GigaChannel #4090

Open
wants to merge 81 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@ichorid
Copy link
Contributor

ichorid commented Dec 6, 2018

This patch replaces QT List-based view of the channels and their contents with a TableView-based widget that uses QT MVC idiom The provided TableModel is capable of loading data lazily from the REST endpoint. The search endpoint is modified to send the data in small pages and do sorts and searches. The TableView serves the model with a custom Delegate. In addition, data from the old Channels implementation can be served through (ugly) dual-stack adapters in the same table.

There is a LOT of bugs to fix and work to do to finish this thing:

  • No unit tests provided! Feel free to add some or fix the old ones.
  • Piles of dead code there. The new system probably made thousands of lines of code obsolete. Please, help in removing it.
  • The new code is horrible. Do refactor as you please.
  • The table appearance actually sucks... Whatever you do to make it look better - please, do it.
  • Etc, etc...

Having said all that, there are only two positive sides about this PR:

  1. It works
  2. It is able to instantly search and display channel with millions of entries (tested).

UPDATE

This PR now includes lots of other changes and fixes related to GigaChannels. At the point of its merging, it will provide everything necessary for GigaChannels to become production-ready.

Things left to do (beside usual requirements like test coverage):

  • Remove "Libnaclpk" prefix from public keys in mdblob format.
  • Add simple differential compression to squashed mdblobs (omit repeated fields after the first mdblob).
  • Fix hover states for "subscribe" button.
  • Add channel size indicator to channel page.
  • Add "xxx" column to Pony DB
  • Add torrent health table
  • Add ability to edit metadata entries
  • Limit metadata size and add an indicator for it to the GUI
  • Add converter for foreign legacy channels
  • Add converter for the user's own legacy channel to proper GigaChannel
  • Add error handling/delays/marking of broken channels
  • Add cleanup/handling of user's channel directory after db cleanup

@ichorid ichorid added this to the V7.2: Gigachannels milestone Dec 6, 2018

@ichorid ichorid requested review from xoriole , devos50 and qstokkink Dec 6, 2018

Show resolved Hide resolved TriblerGUI/tribler_window.py Outdated
Show resolved Hide resolved Tribler/Core/Modules/restapi/channels/channels_torrents_endpoint.py Outdated
Show resolved Hide resolved TriblerGUI/widgets/lazytableview.py Outdated
Show resolved Hide resolved TriblerGUI/widgets/lazytableview.py Outdated
Show resolved Hide resolved Tribler/Core/CacheDB/SqliteCacheDBHandler.py Outdated
@devos50

This comment has been minimized.

Copy link
Contributor

devos50 commented Dec 8, 2018

retest this please

1 similar comment
@devos50

This comment has been minimized.

Copy link
Contributor

devos50 commented Dec 8, 2018

retest this please

@ichorid ichorid force-pushed the f_channelgui branch 2 times, most recently from 3f4d64f to 45a844f Dec 11, 2018

@ichorid

This comment was marked as outdated.

Copy link
Contributor

ichorid commented Dec 12, 2018

Py3k lint list is HUGE for some reason.
Any help on that would be very appreciated...

@ichorid ichorid force-pushed the f_channelgui branch 2 times, most recently from 659855b to 8794bdf Dec 13, 2018

@devos50 devos50 force-pushed the f_channelgui branch from d2e8761 to f5c4d72 Dec 23, 2018

@devos50

This comment has been minimized.

Copy link
Contributor

devos50 commented Dec 28, 2018

For anyone looking at/working on this PR, I am currently working on some major changes to the GUI code and polishing the GUI elements.

@ichorid I reviewed/tested the PR. Overall, I think you did an amazing job on this PR! The pagination works very smoothly and it is a very nice QoL improvement. It really allows the user interface to scale. Although I was a bit worried about the manual drawing of GUI elements, it allows for a huge boost in performance. Also, the code is not hard to understand, and maintainable enough. Also, the way you wrote the code and use of comments indicates your design decisions and helped me to understand your thoughts 👍

That said, I think that code maintainability and structure, both in the core and GUI can even be more improved. I've been working on some changes yesterday and today and I already have a big list of changes I made and some feedback I have for you. Note that these changes are mostly related to code structure (i.e. use of subclassing/splitting different classes into separate files...) and I barely made any change to the functionality, which is overall good I think. I documented each change I made and elaborated why I made it.

The biggest change, for now, is that I split the model/view/delegate logic into separate units that control table rows for displaying channels, and table rows for displaying torrents. You combined this logic but I argue that it is better to split them since these views are fundamentally different. It adds some code but I think it will benefit maintainability in the long term.

One thing that is helping me, is to run the fake Tribler API instead of the 'real' Tribler core. An upcoming change from my side and something that we already discussed is that the RESTful API should really be RESTful (instead of having one /search endpoint that is used to query all resources). To simplify the GUI development process, and to work towards some functional GUI tests, I am refactoring the fake Tribler API backend to support both old-style channels and new-style ones. Hopefully, I will have some functional GUI tests in a day or two.

Finally, it seems that some of the PR checks are only testing code located in the Tribler package. I should adjust this so it also checks code in the TriblerGUI directory. This will probably reveal more pylint issues.

@ichorid

This comment has been minimized.

Copy link
Contributor

ichorid commented Dec 28, 2018

I'm glad you appreciated my humble contribution 😊
Regarding splitting the code for rows into separate units for torrents and channels, my only concern is that we should keep the ability to display different kinds of rows in a single view. It is really important for displaying search results (which are always mixed) and for future developments, such as support for audio tracks, nested channels, PDFs etc.

@ichorid

This comment was marked as resolved.

Copy link
Contributor

ichorid commented Jan 17, 2019

Traceback (most recent call last):
  File "/home/vader/src/TRIBLER/tribler_ichorid/TriblerGUI/event_request_manager.py", line 129, in on_read_data
    raise RuntimeError(json_dict["event"]["text"])
RuntimeError: [Failure instance: Traceback: <type 'exceptions.AttributeError'>: 'buffer' object has no attribute 'encode'
/home/vader/.local/lib/python2.7/site-packages/twisted/web/_newclient.py:1229:_bodyDataFinished_CONNECTED
/home/vader/.local/lib/python2.7/site-packages/twisted/web/client.py:2170:connectionLost
/home/vader/.local/lib/python2.7/site-packages/twisted/internet/defer.py:459:callback
/home/vader/.local/lib/python2.7/site-packages/twisted/internet/defer.py:567:_startRunCallbacks
--- <exception caught here> ---
/home/vader/.local/lib/python2.7/site-packages/twisted/internet/defer.py:653:_runCallbacks
/home/vader/src/TRIBLER/tribler_ichorid/Tribler/Core/TorrentChecker/session.py:337:_process_scrape_response
]

ichorid added some commits Jan 17, 2019

@ichorid

This comment was marked as resolved.

Copy link
Contributor

ichorid commented Jan 17, 2019

Traceback (most recent call last):
  File "/home/vader/src/TRIBLER/tribler_ichorid/TriblerGUI/event_request_manager.py", line 129, in on_read_data
    raise RuntimeError(json_dict["event"]["text"])
RuntimeError: Unhandled Error
Traceback (most recent call last):
  File "/home/vader/.local/lib/python2.7/site-packages/twisted/python/log.py", line 86, in callWithContext
    return context.call({ILogContext: newCtx}, func, *args, **kw)
  File "/home/vader/.local/lib/python2.7/site-packages/twisted/python/context.py", line 122, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/home/vader/.local/lib/python2.7/site-packages/twisted/python/context.py", line 85, in callWithContext
    return func(*args,**kw)
  File "/home/vader/.local/lib/python2.7/site-packages/twisted/internet/posixbase.py", line 614, in _doReadOrWrite
    why = selectable.doRead()
--- <exception caught here> ---
  File "/home/vader/.local/lib/python2.7/site-packages/twisted/internet/udp.py", line 249, in doRead
    self.protocol.datagramReceived(data, addr)
  File "/home/vader/src/TRIBLER/tribler_ichorid/Tribler/Core/TorrentChecker/session.py", line 384, in datagramReceived
    self.tracker_sessions.pop(transaction_id).handle_response(data)
  File "/home/vader/src/TRIBLER/tribler_ichorid/Tribler/Core/TorrentChecker/session.py", line 559, in handle_response
    self.handle_connection_response(response)
  File "/home/vader/src/TRIBLER/tribler_ichorid/Tribler/Core/TorrentChecker/session.py", line 595, in handle_connection_response
    message = struct.pack(fmt, self._connection_id, self.action, self.transaction_id, *self._infohash_list)
struct.error: argument for 's' must be a string

ichorid added some commits Jan 17, 2019

@ichorid

This comment has been minimized.

Copy link
Contributor

ichorid commented Jan 18, 2019

Some stats on converter:
Original database (tribler.sdb): 394M - > Pony database (metadata.db) 70M
77 000 entries converted in 269 seconds (288 entries/second)

@ichorid

This comment was marked as resolved.

Copy link
Contributor

ichorid commented Jan 18, 2019

When clicked on sort header above "buttons" column:

Traceback (most recent call last):
  File "/home/vader/src/TRIBLER/tribler_ichorid/TriblerGUI/widgets/triblertablecontrollers.py", line 66, in _on_view_sort
    self.load_search_results(1, 50)
  File "/home/vader/src/TRIBLER/tribler_ichorid/TriblerGUI/widgets/triblertablecontrollers.py", line 89, in load_search_results
    + (('&type=%s' % self.model.type_filter) if self.model.type_filter else ''),
TypeError: %d format: a number is required, not NoneType
@ichorid

This comment was marked as resolved.

Copy link
Contributor

ichorid commented Jan 18, 2019

When clicking on the empty space where "download" button should be in "search" table, for a channel row:

Traceback (most recent call last):
  File "/home/vader/src/TRIBLER/tribler_ichorid/TriblerGUI/widgets/lazytableview.py", line 49, in on_download_button_clicked
    self.window().start_download_from_uri(index2uri(index))
  File "/home/vader/src/TRIBLER/tribler_ichorid/TriblerGUI/utilities.py", line 13, in index2uri
    infohash = index.model().data_items[index.row()][u'infohash']
KeyError: u'infohash'
@ichorid

This comment has been minimized.

Copy link
Contributor

ichorid commented Jan 18, 2019

Problem: Subscribe icon does not change state when clicked.

@ichorid ichorid force-pushed the f_channelgui branch 2 times, most recently from 46f7f71 to 6746594 Jan 21, 2019

@ichorid

This comment was marked as resolved.

Copy link
Contributor

ichorid commented Jan 21, 2019

Problem: after the first click on "apply changes" button torrents controls become unclickable.

@ichorid ichorid force-pushed the f_channelgui branch from 6746594 to 6345382 Jan 21, 2019

ichorid added some commits Jan 21, 2019

@ichorid

This comment has been minimized.

Copy link
Contributor

ichorid commented Jan 21, 2019

Another database converted:
294363 entries converted in 708 seconds (415 e/s)

This time there was a couple of really malformed entries, so try-except had to be added in the conversion loop to chew on these.

@ichorid

This comment has been minimized.

Copy link
Contributor

ichorid commented Jan 21, 2019

Performance increase is most probably due to Pony upgrade ))

@ichorid

This comment was marked as resolved.

Copy link
Contributor

ichorid commented Jan 22, 2019

on search

Traceback (most recent call last):
  File "/home/vader/src/TRIBLER/tribler_ichorid/TriblerGUI/widgets/torrentdetailstabwidget.py", line 72, in on_torrent_info
    self.check_torrent_health()
  File "/home/vader/src/TRIBLER/tribler_ichorid/TriblerGUI/widgets/torrentdetailstabwidget.py", line 124, in check_torrent_health
    self.index.model().data_items[self.index.row()][u'health'] = HEALTH_CHECKING
IndexError: list index out of range

ichorid added some commits Jan 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment