-
Notifications
You must be signed in to change notification settings - Fork 109
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
Add command to log in to Plex #230
Conversation
04743f1
to
311188d
Compare
@BEisem can you test this? it doesn't save them yet to config (only prints), but I'm interested is the UX suitable. |
Please don't hate me, but I'm not sure what you want me to test. I'm not a programmer so I'm trying to look at the changes in the code, but I don't understand what I'm looking at. If you let me know what to test, I'll be happy to do it... thanks! |
@BEisem I'm asking to test the command, not look at the code! the command usage is printed in PR body! |
Ok, I've tried this out and it's not working for me. I'm getting
I tried it with both I know I'm probably doing something wrong, just not sure what... |
you need to use zip from the branch of this pull request if you intend to test code from this pull request, it's if you are getting no such command error, you did not use my branch. I tested exactly those steps right now, and it does have the command. you need additionally to copy |
Ok! Now I got it ... thank you. I went through a few options and it works nicely. I purposely entered an incorrect password, and instead of crashing it told me the login failed. I think this is a much better overall setup experience. The only thing I noticed so far is, I decided I wanted to quit during the setup process, and I tried CTRL-C but it didn't break. It just repeated the last command. So I had to complete the process before I was able to get out of it. Also, after the setup is complete, instead of starting the first sync, it just exits back to the command line. Was this intentional? |
@BEisem yes, this is intended as a standalone command (notice that command is named 'plex-login'). with a single purpose: login to plex. yeah, I also noticed that ctrl+c does not abort the process, this may be something internal to the you can |
ok, the ctrl+c is likely my fault: should never ever catch top-level I added the top-level exception temporarily because could not figure out what exceptions plex.connect() can throw. it seemed not. to throw their own exceptions but exceptions came from httplib. |
@BEisem: Removed catching plain |
Ok I'm not sure what I'm doing wrong here, but I'm sure it's me... I downloaded the zip file from |
@BEisem zip url is correct. so redo steps past that. rm -rf and unpack zip again. executed from wrong dir? unpacked wrong copy of the zip. dunno really. |
I don't know what to tell you ... I've done this multiple times, tried it on several different computers, downloaded from the above URL, unzip to a new directory, enter that directory and run the command. I'm 100% sure I am not executing from the wrong dir, haven't unpacked the wrong copy, etc. I give up... |
@BEisem show your error, exact steps, I can't help you with just blind guessing. |
Results in the old-style setup procedure, not the new one. |
@BEisem seems you have forgotten that I said you need to copy the |
I've added .env save to the end of the method, so this command is complete now. NOTE: The removal of old setup is not removed in this PR context, trakt login command needs to be added first. |
Do not exit the loop until result is validated
the command line options are optional, they are asked interactively.
Similarly to #243