BenjV / autosub-bootstrapbill

Bootstrapbill version of autosub
25 stars 11 forks source link

Added notification support for the new Plex server #23

Closed ash-vd closed 8 years ago

ash-vd commented 8 years ago

For the latest version of PMS, we need an access token to update the library. Also, you can now update just a single library, so we don't update the whole movies library when we only need to update the tv shows.

BenjV commented 8 years ago

Je kunt niet zomaar urllib vervangen door het woord requests dat werkt zo niet. Je hebt het dus waarschijnlijk niet getest?

Voor de rest ben je behoorlijk grondig geweest, de meeste mensen zouden wel iets vergeten zijn aan te passen, petje af.

ash-vd commented 8 years ago

Ik heb 't niet 1 op 1 vervangen hoor, o.a. de headers worden nu anders meegegeven. Ik heb het getest met m'n eigen Plex server, en daar werkt het naar behoren. De oude versie heb ik niet draaien, dus dat heb ik niet getest... De syntax van urllib(2) en requests komt redelijk overeen voor dergelijke simpele calls. De meest ingewikkelde is de POST om de token op te halen, verder zijn het simpele GET requests met de parameters in de URL verwerkt.

BenjV commented 8 years ago

Sorry ik keek even verkeerd. Kwam doordat ik naar de diff keek en dan is het wel eens verwarrend welke de juiste regel is.

Je moet nog wel even een include toevoegen voor de systemen die geen requests als deel van de pyhon library op hun systeem hebben staan. Autosub neemt z'n eigen requests library mee, dan weet ik dat het altijd de goede versie is.

Deze include dus:

import library.requests as requests

ash-vd commented 8 years ago

Die diff is inderdaad soms verwarrend, zeker als er grote blokken code veranderen.. Ik heb de import veranderd op de manier zoals je aangaf, en getest.

BenjV commented 8 years ago

Ik heb hem gemerged met de main branch. Ik ben zelf ook met een paar wijzigingen bezig en als die gereed zijn vanmiddag zal ik het versienummer ophogen zodat iedereen kan updaten vanuit autosub.

ash-vd commented 8 years ago

Top, thanks voor de snelle merge!

BenjV commented 8 years ago

Jij bedankt voor de code. Ik kan niet notifications niet zelf aanpassen, omdat ik ze niet kan testen zonder zoals in dit geval een plexmedia server.

ash-vd commented 8 years ago

Zou je misschien op regel 12 en 20 van plexmediaserver.py de log entry nog kunnen updaten naar het juiste formaat? Zag dat ik die vergeten ben, en om daar nou een hele nieuwe pull request voor aan te maken vind ik ook zo overdreven.

BenjV commented 8 years ago

Neem ik mee met de volgende update. Is niet echt belangrijk.