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 Utils::Internet.connected? to determine whether host machine has internet connection. #5870

Merged
merged 3 commits into from Nov 2, 2017

Conversation

@parkr
Member

parkr commented Feb 8, 2017

This PR adds a POC for a common API for checking whether the user has an
Internet connection wherever they're running Jekyll.

Many plugins make calls out into the Internet for data. In order to facilitate a pattern
for gracefully falling back when there is no Internet connection for the user, this module
and module function are added. Plugins which access the Internet should decide whether there
is a path for them to gracefully fallback to some non-Internet default when the host computer
is not connected.

A lot of the work we're doing with GitHub Pages (jekyll/github-metadata,
and things like github/pages-gem#408) require
the internet. We'd like to be able to fall back gracefully when there's
no internet connection, and a common API makes the most sense going
forward.

  • Decide on logical timeouts (connect & open timeouts) and implement
  • Add more robust tests

/cc @jekyll/ecosystem

Add Utils::Internet.connected? to determine whether host machine has …
…internet connection.

Many plugins make calls out into the Internet for data. In order to facilitate a pattern
for gracefully falling back when there is no Internet connection for the user, this module
and module function are added. Plugins which access the Internet should decide whether there
is a path for them to gracefully fallback to some non-Internet default when the host computer
is not connected.

@jekyllbot jekyllbot self-assigned this Feb 8, 2017

@parkr parkr added the feature label Feb 8, 2017

@parkr parkr assigned benbalter and unassigned jekyllbot Feb 8, 2017

Show outdated Hide outdated lib/jekyll/utils/internet.rb Outdated

@benbalter benbalter removed their assignment Feb 27, 2017

parkr added some commits Nov 2, 2017

@parkr parkr requested review from jekyll/core and DirtyF Nov 2, 2017

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Nov 2, 2017

Member

This is good.

I wonder if, in the future, we should add an option to never connect to the internet. Maybe set an environment variable to always return false.

One limitation of this approach is that it will appear there is an internet connection if the user is connected but not authenticated to a hotspot that has a gateway where every request returns a webpage asking the user to agree to terms of service. I think, for our purposes, that is probably not a very big deal.

Member

pathawks commented Nov 2, 2017

This is good.

I wonder if, in the future, we should add an option to never connect to the internet. Maybe set an environment variable to always return false.

One limitation of this approach is that it will appear there is an internet connection if the user is connected but not authenticated to a hotspot that has a gateway where every request returns a webpage asking the user to agree to terms of service. I think, for our purposes, that is probably not a very big deal.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Nov 2, 2017

Member

I wonder if, in the future, we should add an option to never connect to the internet. Maybe set an environment variable to always return false.

That's a good idea. This check doesn't mean we can actually make a TCP connection anywhere useful, but at least we can get situations where a user's wifi is turned off, etc.

@jekyllbot: merge +minor

Member

parkr commented Nov 2, 2017

I wonder if, in the future, we should add an option to never connect to the internet. Maybe set an environment variable to always return false.

That's a good idea. This check doesn't mean we can actually make a TCP connection anywhere useful, but at least we can get situations where a user's wifi is turned off, etc.

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 8fc463b into master Nov 2, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot deleted the internet-connected branch Nov 2, 2017

@DirtyF DirtyF added this to the v3.7.0 milestone Nov 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment