-
Notifications
You must be signed in to change notification settings - Fork 200
Sync support #282
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
Sync support #282
Conversation
|
Hi! Thanks for the PR! How do you actually sync the items? Can you add a test? I would expect that there was a sync function inside the client so you could just pass a list of objects and that would be it. Any info needed for the transcode can be picked up there this way we don’t need to mess with the init method. |
|
Just to clarify: the PR is not finished now, it's an early alpha version :)
The list of "items-to-sync" is controlled in Plex Web, you can add any item to the list for a specific device. In the following code block I will show the way to download all suitable content (actually I've not yet reached the "downloading" stage in my project, so I'm not sure if this really working now):
Yep, of course, on some latter stage :)
As far as I can see the only way for Plex to detect transcoding options, is thru target platform. Here is what I see in the Plex logs with real platform info: And the process goes smoothly when I mimic my iPhone SE. Also here is the request which should be sent to add an item to sync list: As you can see there are nothing here about container and/or supported codecs list, only basic/common transcoding stuff. |
|
You dont really need to do anything to make the sync happens. Seems you just upload zhe shit to a queue, and the client handles the rest from there. Anyway to send a sync command should be as easy as: inside client.py Looking forward to test the pr :) |
Yep, adding new sync-item to some other client is easy, and you don't have to call sync.init() for it. And it's not supported by this PR now :) My goal is to support "sync-target": when you run the script on some random computer, select sync-items in your Plex Web and the required media magically being copied to this computer, in the suitable format. In my case I have an external HDD and I'd like to always have some movies / tv shows (converted to mp4 and not in 4k format), e.g. to be able to watch something while I'm on a plane. |
|
I dont think syncing to computers is supported by plex at all. Do you just want to copy things from another shared plex server to your own? Or sync to a actual client? Anyway if you just want to download stuff or transcode to a computer thats already supported. |
It's not supported out-of-the-box, but we're the engineers, and we can force software to do what we want! :)
I want to copy optimised copies of things from any plex server to an external hdd |
|
Oh, thats easy. Its pretty much supported out of the box. See the docs for. note the comment about **kwargs – Additional options passed into getStreamURL(). here was a example on how i use it from one my my own projects. https://github.com/Hellowlol/plex-cli/blob/master/plexcli/cli.py#L126 |
|
Yep, I saw this option, I plan to support it in my project, but it's not on my priority list...
|
|
I think im missing something here. Do you expect to do this from the plex ui? |
You're exactly right :) https://support.plex.tv/articles/201082477-quick-guide-to-mobile-sync/
Man... I just like the Mobile Sync approach, where you don't have to code the filters by yourself, but you can easily choose the required things with UI and this UI doesn't require any dev-knowledge at all, even my wife would be able to use it! :D |
|
Oh, i think i get it now. You faking so plexapi is detected as a client in the plex webui... imo that should be done with your own project. Nice idea btw! |
You mean |
When you have multiple media within one SyncItem it takes a lot of time to get all the info for this media (on my machine it takes about a second for each movie).
plexapi/base.py
Outdated
| on how this is used. | ||
| """ | ||
| data = self._server.query(ekey) | ||
| timeout = kwargs.pop('timeout', 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.
This should probably be added as a real keyword arg timeout=None in the function definition and not pulled out of kwargs.
plexapi/myplex.py
Outdated
|
|
||
|
|
||
| def _connect(cls, url, token, timeout, results, i): | ||
| def _connect(cls, url, token, timeout, fast, results, i, connected_event=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.
If X_PLEX_ENABLE_FAST_CONNECT is a global configuration option. Should we instead always use fast=True or false based on that, rather than passing it in as an argument?
plexapi/utils.py
Outdated
| for thread in threads: | ||
| thread.join() | ||
| return results | ||
| while not connected_event.is_set(): |
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.
Just curious, what was the issue that led to this change?
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.
Most times I run the my things outside my home network, when the local IP of the server is not available, and it takes annoyingly long time to establish the connection
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.
Makes sense, thanks.
|
@mikes-nasuni thank you for the fast reaction, I've made the changes which you've suggested :) |
plexapi/utils.py
Outdated
| if all([not t.is_alive() for t in threads]): | ||
| break | ||
| time.sleep(0.05) | ||
| return list(filter(lambda r: r is not None, results)) |
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.
Use a list comprehension
plexapi/sync.py
Outdated
| url = '/sync/%s/%s/files/%s/downloaded' % ( | ||
| self._device.clientIdentifier, server.machineIdentifier, sync_id) | ||
| server.query(url, method=requests.put) | ||
| def markDownloaded(self, media: Playable): |
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.
This need to be support python 2.
plexapi/sync.py
Outdated
| def server(self): | ||
| server = list(filter(lambda x: x.machineIdentifier == self.machineIdentifier, self._servers)) | ||
| server = list(filter(lambda x: x.clientIdentifier == self.machineIdentifier, self._server.resources())) | ||
| if 0 == len(server): |
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.
List comp please
plexapi/sync.py
Outdated
| return server[0] | ||
|
|
||
| def getMedia(self): | ||
| def getMedia(self, timeout=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.
What up with the timeout? Why not just change the plex.Timeout?
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.
Not all operations require 60s+ timeout, and wrapping getMedia() calls with changing timeout back-and-forth would look strange.
You think that just globally change TIMEOUT in my code would be more than enough and wouldn't cause any strange problems?
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 think that just globally change TIMEOUT in my code would be more than enough and wouldn't cause any strange problems?
It shouldn't cause any problems. timeout is essentially just telling the requests module when to give up if not getting a response. It's really only useful to change to poor or slow network issues. Having different timeouts for different calls in to the API seems incorrect unless you have a really strong case for it.
Perhaps a better option for your case is to update your personal config file which is not checked in.
plexapi/myplex.py
Outdated
| """ Returns an instance of :class:`~plexapi.sync.SyncItems` for specified client | ||
| Arguments: | ||
| clientId (str): an identifier of a client for which you need to get SyncItems. Would be set to current |
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.
allow for both a client and a clientid as a string.
plexapi/myplex.py
Outdated
| """ Adds specified sync item for the client | ||
| Arguments: | ||
| client (:class:`~plexapi.myplex.MyPlexDevice`): pass |
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.
doc what this client, sync_item is used for.
| return '%s%s' % (self._baseurl, key) | ||
|
|
||
| def refreshSynclist(self): | ||
| return self.query('/sync/refreshSynclists', self._session.put) |
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.
add missing doc string
| return self.query('/sync/refreshSynclists', self._session.put) | ||
|
|
||
| def refreshContent(self): | ||
| return self.query('/sync/refreshContent', self._session.put) |
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.
add missing doc string
plexapi/sync.py
Outdated
| status (:class:`~plexapi.sync.Status`): current status of the sync | ||
| mediaSettings (:class:`~plexapi.sync.MediaSettings`): media transcoding settings used for the item | ||
| policy (:class:`~plexapi.sync.Policy`): the policy of which media to sync | ||
| location (str): unknown |
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.
self.clientidentifier is not listed as a attribute.
plexapi/sync.py
Outdated
| """ Returns :class:`~plexapi.myplex.MyPlexResource` with server of current item. | ||
| """ | ||
| server = [s for s in self._server.resources() if s.clientIdentifier == self.machineIdentifier] | ||
| if 0 == len(server): |
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.
can you do len(server) == 0 instead,
| media._server.query(url, method=requests.put) | ||
|
|
||
| def delete(self): | ||
| url = SyncList.key.format(clientId=self.clientIdentifier) |
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.
Missing doc string
plexapi/sync.py
Outdated
| self.clientId = data.attrib.get('clientIdentifier') | ||
| self.items = [] | ||
|
|
||
| for elem in data: |
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.
Can we use .iter('SyncItem') here?
plexapi/video.py
Outdated
| return self._prettyfilename() | ||
|
|
||
| def sync(self, client, policy, media_settings, title=None): | ||
| """ Add current video as sync item for specified device. |
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.
add docs for args, and keywork arguments.
tests/test_sync.py
Outdated
| MAX_RETRIES = 3 | ||
|
|
||
|
|
||
| def get_new_synclist(account, client_id, old_sync_items=[]): |
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.
old_sync_items is mutable, is this intended?
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've already rewrote this part locally, anyway :D
|
Heyhey, @Hellowlol, thank you for the review! I've pushed another pile of code, where I've done few things:
In order to get sync/tests for photos working PR #292 need to be applied. |
|
Let me know when this is ready, i want to squash and merge this before i do the small prs. |
|
Yep, sure. I hope to finish this by tomorrow. |
|
@Hellowlol here goes the docs :) Anything else you'd like to be changed? |
|
Thanks for the doc changes. I havnt really looked at the code before now. There is a lot of repetitions regarding the sync methods. Anyway we could simplify this? |
|
Uh... From the top of my head — I can add a mixin @Hellowlol Do you have any better ideas? |
|
Using a mixin was my thought too. If there are too many options we might just leave it as it is. Anyway are you done? |
|
Yep, it's done I think. Probably I'll find something else later, but for now I have nothing to add :) |
|
Awesome, thanks! |
|
Can you fix the inline imports so the style is consistent? We don't seem to use any other relative imports, |
|
Yep, sure, I've just pushed it |
Hi there! I've started working on Mobile Sync support. If anybody has any comments on the code, or just any feature requests — feel free to tell :)
At the beginning the code can do few useful things:
syncItemIdThe reason why I'm started this is simple: I have a WD My Passport Pro which has the Plex onboard and I'd like to have my "travel" library in sync with the main one. I've already started the project for this (andrey-yantsen/plexiglas) but for now there is nothing to see there.
fix #134