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

[Batch 1] Port Notebook PRs #95

Merged
merged 8 commits into from
Sep 25, 2019

Conversation

vit-tucek and others added 8 commits September 25, 2019 08:42
to ensure (approximately) that all messages from the stopped kernel are delivered before the dead/restarting message
Per Tornado's documentation:

>By default, Tornado’s secure cookies expire after 30 days.
>To change this, use the expires_days keyword argument to
>set_secure_cookie and the max_age_days argument to get_secure_cookie.
>These two values are passed separately so that you may
>e.g. have a cookie that is valid for 30 days for most purposes,
>but for certain sensitive actions
>(such as changing billing information)
>you use a smaller max_age_days when reading the cookie.

With the current implementation in `auth/login.py`,
this is possible to pass the `expires_days` option
but not possible to enforce it as this is not possible
to pass `max_age_days` to `get_secure_cookie`

This makes impossible to set the cookie expiration without
using a custom `LoginHandler`.

This revision is about adding the possibility to pass options
to Tornado's `get_secure_cookie` method,
so it can be possible to set the cookies expiration,
among others.
ipaddress.ip_address('127.0.0.1') fails with ValueError on Python 2

need to decode it, otherwise 127.0.0.1 won't be treated as local
so this list isn't empty when these handlers are used outside NotebookApp
@Zsailer Zsailer changed the title Port notebook PRs 3664, 3759, 3778, 3809 Port notebook PRs 3664, 3759, 3778, 3809, 3829 Sep 25, 2019
@vidartf
Copy link
Member

vidartf commented Sep 25, 2019

Some of these seem to undo some things from #94 ?

@vidartf
Copy link
Member

vidartf commented Sep 25, 2019

Or rather, contrary to that PR.

@Zsailer
Copy link
Member Author

Zsailer commented Sep 25, 2019

You're right. This is the challenge of cherry-picking PRs. I'll remove the Python2/3 patches in a new commit (rather than removing specific commits). Does that seem reasonable?

@kevin-bates
Copy link
Member

Shoot. We should probably complete the initial migration from Notebook before doing "new" things. Sorry about that. Nice catch @vidartf.

@Zsailer Zsailer changed the title Port notebook PRs 3664, 3759, 3778, 3809, 3829 [Batch 1] Port Notebook PRs Sep 25, 2019
@@ -419,6 +419,10 @@ def check_host(self):
if host.startswith('[') and host.endswith(']'):
host = host[1:-1]

if not PY3:
Copy link
Member Author

Choose a reason for hiding this comment

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

(#96) We dropped support for Python 2 in #94.

@@ -219,7 +219,7 @@ def init_settings(self, jupyter_app, kernel_manager, contents_manager,
now = utcnow()

root_dir = contents_manager.root_dir
home = os.path.expanduser('~')
home = py3compat.str_to_unicode(os.path.expanduser('~'), encoding=sys.getfilesystemencoding())
Copy link
Member Author

Choose a reason for hiding this comment

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

(#96) We dropped support for Python 2 in #94.

@kevin-bates kevin-bates merged commit a8022ab into jupyter-server:master Sep 25, 2019
@kevin-bates
Copy link
Member

#96: Remove commit 349045e - ip_address only accepts unicode on Python 2

@Zsailer Zsailer deleted the notebook-3664 branch January 10, 2020 17:35
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.

7 participants