Skip to content
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

new login flow #209

Merged
merged 1 commit into from
Oct 5, 2018
Merged

new login flow #209

merged 1 commit into from
Oct 5, 2018

Conversation

houqp
Copy link
Contributor

@houqp houqp commented Oct 4, 2018

No description provided.

@houqp houqp requested a review from narenst October 4, 2018 00:26
@houqp houqp force-pushed the login branch 6 times, most recently from 5db61ec to 262c3d2 Compare October 4, 2018 18:02
subprocess.check_output(
[sys.executable, '-m', 'webbrowser', url], stderr=subprocess.STDOUT)

floyd_logger.info('waiting for login from browser...')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be printed earlier than this - may be at the top of this function.

When I ran login, there was no output for a few seconds and then the browser opened - I did not see this message until I came back to the terminal.

from queue import Queue


class LoginServer(HTTPServer, object):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The web server code can be moved to a its own file under managers.

floyd/cli/auth.py Outdated Show resolved Hide resolved
floyd/cli/auth.py Show resolved Hide resolved
@houqp houqp force-pushed the login branch 2 times, most recently from 4756d55 to 28779b7 Compare October 5, 2018 15:47
@houqp houqp merged commit 48d8949 into floydhub:master Oct 5, 2018
@houqp houqp deleted the login branch October 5, 2018 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants