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

Automatic release update warnings. #1336

Merged
merged 7 commits into from
May 5, 2022
Merged

Conversation

isidentical
Copy link
Contributor

@isidentical isidentical commented Mar 29, 2022

This PR embeds an automatic update warning system into the HTTPie. What this means is that, on some configurable thresholds (once a week) HTTPie will check whether a new release is available and if it there is it will give user a warning.

Here is an example:
image

This problem consists of two dimensions, which each need to be handled separately:

  1. Correctly (without any false positives) detect whether there is a new release is available.
  2. Ensure there is no observable startup penalty.

Update detection

Since HTTPie is distributed on many different channels (from Homebrew to Snap, PyPI to Chocolatey) we need to be careful to show these warnings to only users whom can upgrade at that moment. So for that, we have a new file called httpie/internal/_build_channel.py. This file has no runtime purpose, other than contain the BUILD_CHANNEL constant.

The default for that constant is unknown, and each build system will overwrite it with the value pointing to their name. E.g on Homebrew, during the build process, we'll write BUILD_CHANNEL = 'brew' into that file. This way, we can detect whether the build is from Homebrew or not.

Once we know what build channel the user is using, we need to determine whether we have a new release for that channel. That is where packages.httpie.io/latest.json comes into place. It is an automatically updated (in a daily basis) registry of build channels and the HTTPie versions in them.

We compare the current version of HTTPie with the latest version for the build channel by looking to the latest.json. If there is a new release, we show the warning.

Startup penalty

For reducing the overhead of this process as much as we can, we do it in two steps.

  • Fetch the latest.json into a local file (~/.config/httpie/version_info.json) (done in a separate process, takes a long time)
  • Warn the user (done in the same process, takes literally no time <1ms)

For the first task, this PR introduces a daemon system where we can start tasks in the background and detach them from the main process. So even if you run a short HTTPie command (e.g http --version), you exit immediately and the actual task still works to finish up its objective without blocking you.

This part is implemented by using double-forking on POSIX, and creating a subprocess on Windows.

Baseline

Master:

BASELINE (master)# hyperfine 'http --offline pie.dev/get' 
Benchmark 1: http --offline pie.dev/get
  Time (mean ± σ):     217.7 ms ±   4.8 ms    [User: 199.0 ms, System: 18.1 ms]
  Range (min … max):   210.1 ms … 225.0 ms    13 runs

This branch:

(this branch)# hyperfine 'http --offline pie.dev/get'
Benchmark 1: http --offline pie.dev/get
  Time (mean ± σ):     217.0 ms ±   5.4 ms    [User: 195.7 ms, System: 21.0 ms]
  Range (min … max):   210.3 ms … 226.9 ms    13 runs

(this branch)# hyperfine 'rm ~/.config/httpie/version_info.json; http --offline pie.dev/get'
Benchmark 1: rm ~/.config/httpie/version_info.json; http --offline pie.dev/get
  Time (mean ± σ):     236.8 ms ±  14.5 ms    [User: 205.3 ms, System: 30.9 ms]
  Range (min … max):   213.7 ms … 253.3 ms    11 runs

It seems like once the version_info file is created, there is no penalty. When the file does not exist, there is a very small cost of forking the process which is ~15 ms (you will pay this cost twice a month, so if you ran HTTPie 1000 times that month this will cost ~30 ms in overall overhead).

@isidentical isidentical marked this pull request as ready for review April 1, 2022 11:19
@isidentical
Copy link
Contributor Author

This is blocked by #1330

@isidentical isidentical marked this pull request as draft April 14, 2022 07:50
@isidentical isidentical force-pushed the auto-updates branch 3 times, most recently from f44b62d to be59d98 Compare April 18, 2022 12:26
@codecov-commenter
Copy link

codecov-commenter commented Apr 18, 2022

Codecov Report

Merging #1336 (6e75c52) into master (4d7d6b6) will decrease coverage by 2.10%.
The diff coverage is 92.68%.

@@            Coverage Diff             @@
##           master    #1336      +/-   ##
==========================================
- Coverage   97.28%   95.17%   -2.11%     
==========================================
  Files          67      109      +42     
  Lines        4235     7633    +3398     
==========================================
+ Hits         4120     7265    +3145     
- Misses        115      368     +253     
Impacted Files Coverage Δ
httpie/output/ui/man_pages.py 0.00% <0.00%> (ø)
httpie/output/ui/rich_utils.py 0.00% <0.00%> (ø)
tests/test_binary.py 100.00% <ø> (ø)
tests/test_update_warnings.py 99.14% <ø> (ø)
tests/test_uploads.py 96.70% <ø> (-3.30%) ⬇️
tests/test_xml.py 97.56% <ø> (-0.06%) ⬇️
tests/utils/__init__.py 92.37% <ø> (+3.69%) ⬆️
tests/utils/http_server.py 99.00% <ø> (ø)
tests/utils/matching/parsing.py 97.95% <ø> (-2.05%) ⬇️
tests/utils/matching/test_matching.py 100.00% <ø> (ø)
... and 99 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7c1bb2...6e75c52. Read the comment docs.

@isidentical isidentical force-pushed the auto-updates branch 2 times, most recently from 61df919 to 6476ed9 Compare April 18, 2022 13:10
@isidentical isidentical marked this pull request as ready for review April 19, 2022 10:34
@isidentical isidentical requested a review from jkbrzt April 19, 2022 10:35
@isidentical isidentical force-pushed the auto-updates branch 2 times, most recently from f698663 to 34175e2 Compare May 5, 2022 15:24
Comment on lines 1 to 5
# Represents the packaging method. This file should
# be overridden by every build system we support on
# the packaging step.

BUILD_CHANNEL = 'unknown'
Copy link
Member

Choose a reason for hiding this comment

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

Why is the package called internal?

Copy link
Member

Choose a reason for hiding this comment

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

What do you think about calling this module __build_channel__.py to indicate it’s very special?

Comment on lines 11 to 14
# This module provides an interface to spawn a detached task to be
# runned with httpie.internal.daemon_runner on a separate process. It is
# based on DVC's daemon system.
# https://github.com/iterative/dvc/blob/main/dvc/daemon.py
Copy link
Member

Choose a reason for hiding this comment

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

"""Looks like a good docstring candidate"""

There’re a few other comment-to-docstring opportunities in this PR.

@@ -149,6 +149,24 @@ def __init__(self, directory: Union[str, Path] = DEFAULT_CONFIG_DIR):
def default_options(self) -> list:
return self['default_options']

def _configured_path(self, config_option: str, default: str) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

It’d prefix the name with get_

os.path.dirname(os.path.dirname(file_path))
)

if os.name == 'nt':
Copy link
Member

Choose a reason for hiding this comment

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

Could we use httpie.compat.is_windows here?

Comment on lines 106 to 111
if os.name == 'nt':
# Windows lacks os.fork(), so we need to
# handle it separately.
_spawn_windows(args, process_context)
else:
_spawn_posix(args, process_context)
Copy link
Member

Choose a reason for hiding this comment

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

This might be asking for a new _spawn(args, process_context) that would encapsulate the OS check.

@jkbrzt jkbrzt merged commit 003f209 into httpie:master May 5, 2022
egleston pushed a commit to egleston/httpie that referenced this pull request May 5, 2022
* Hide pretty help

* Automatic release update warnings.

* `httpie cli check-updates`

* adapt to the new loglevel construct

* Don't make the pie-colors the bold

* Apply review feedback.

Co-authored-by: Jakub Roztocil <[email protected]>
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.

4 participants