-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
No session resumption on different phone number #56
base: master
Are you sure you want to change the base?
No session resumption on different phone number #56
Conversation
…al file and start a new websession if so. Added command line argument to manage saveing of the credential file without asking.
pytr/account.py
Outdated
''' | ||
If web is true, use web login method as else simulate app login. | ||
Check if credentials file exists else create it. | ||
If no parameters are set but are needed then ask for input | ||
''' | ||
log = get_logger(__name__) | ||
save_cookies = True | ||
save_credentials = (save_credentials == 'y') |
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.
Why not just use a boolean directly?
pytr/main.py
Outdated
@@ -50,6 +50,7 @@ def formatter(prog): | |||
parser_login_args.add_argument('--applogin', help='Use app login instead of web login', action='store_true') | |||
parser_login_args.add_argument('-n', '--phone_no', help='TradeRepublic phone number (international format)') | |||
parser_login_args.add_argument('-p', '--pin', help='TradeRepublic pin') | |||
parser_login_args.add_argument('-s', '--save_credentials', choices=['y', 'n'], help='Save credentials') |
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.
Same here. I'd just remove choices and only save the credentials if the argument is passed (with --save_credentials
)
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 made the change to bool now.
Should I remove the pre-existing ask_for_save code as well and only save them if the switch was used?
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.
Thanks for your PR. I can merge after the argument is changed to bool
…als is used as command line argument.
…n(). They are then also passed to TradeRepublicApi().
Done. I also now added support to credentials and cookies files arguments. |
Using the current code, it seems that the active session is not correctly reused: pytr portfolio |
Added code to compare given phone number with the one in the credential file and disable websession resumption if so.
For this to work well I also added a command line option to select if credentials should be saved or not.
As a bonus a fix for corrupt cookie files got added.
This pull-request fixes #38.