prjemian / punx

Python Utilities for NeXus HDF5 files
https://prjemian.github.io/punx
5 stars 7 forks source link

update: warning issued when token provided #179

Closed prjemian closed 2 years ago

prjemian commented 2 years ago

The update subcommand expects a GitHub token to be provided either via an environment variable (GH_TOKEN or GITHUB_TOKEN) or via a command-line option -t. In the latter case, a warning is issued when the -t option is used (with accompanying token) but no environment variable is found:

(bluesky_2022_1) prjemian@zap:~$ punx up -t ${my_token} -r main

!!! WARNING: this program is not ready for distribution.

============= ======= ====== =================== ======= ==================================================================
NXDL file set type    cache  date & time         commit  path                                                              
============= ======= ====== =================== ======= ==================================================================
a4fd52d       commit  source 2016-11-19 01:07:45 a4fd52d /home/prjemian/Documents/projects/prjemian/punx/punx/cache/a4fd52d
v3.3          release source 2017-07-12 17:41:13 9285af9 /home/prjemian/Documents/projects/prjemian/punx/punx/cache/v3.3   
v2018.5       release source 2018-05-15 16:34:19 a3045fd /home/prjemian/Documents/projects/prjemian/punx/punx/cache/v2018.5
============= ======= ====== =================== ======= ==================================================================

[INFO 2021-12-26 08:56:45.819 punx.main:259] install_NXDL_file_set(ref=main, force=False, user_cache=True)
/home/prjemian/Documents/projects/prjemian/punx/punx/github_handler.py:80: UserWarning: Did not find environment variables GH_TOKEN or GITHUB_TOKEN which provide the GitHub API token necessary to download resources from GitHub through its API.  You may experience restrictions on the amount of content that can be downloaded over a short interval (such as an hour or so).
  warnings.warn(
installed in: /home/prjemian/.config/punx/main
============= ======= ====== =================== ======= ==================================================================
NXDL file set type    cache  date & time         commit  path                                                              
============= ======= ====== =================== ======= ==================================================================
a4fd52d       commit  source 2016-11-19 01:07:45 a4fd52d /home/prjemian/Documents/projects/prjemian/punx/punx/cache/a4fd52d
v3.3          release source 2017-07-12 17:41:13 9285af9 /home/prjemian/Documents/projects/prjemian/punx/punx/cache/v3.3   
v2018.5       release source 2018-05-15 16:34:19 a3045fd /home/prjemian/Documents/projects/prjemian/punx/punx/cache/v2018.5
main          branch  user   2021-12-17 21:09:18 041c2c0 /home/prjemian/.config/punx/main                                  
============= ======= ====== =================== ======= ==================================================================

This is a BUG.

prjemian commented 2 years ago

Use of the token supplied by -t is shallow (limited to the punx.main module) and not propagated further. By the time it is needed later, to actually download content, it appears to be forgotten.

prjemian commented 2 years ago

From a security view, it is best not to keep the token in memory any longer than necessary and the code appears to favor this consideration. The token is used here: https://github.com/prjemian/punx/blob/f7e8a129d107c9dd8b45dabd94bd7c04c49b07e0/punx/main.py#L274-L276

Interesting, this code does not check the environment if the -t option is not used. This is a second BUG (of omission, this time).

prjemian commented 2 years ago

Note, that in line 280 here, force=True will always be the case due to the test at line 274. https://github.com/prjemian/punx/blob/f7e8a129d107c9dd8b45dabd94bd7c04c49b07e0/punx/main.py#L274-L280

prjemian commented 2 years ago

Is it possible to update the cache without a GitHub API token? Might be possible by downloading a ZIP of the file_set and removing the unnecessary content.

prjemian commented 2 years ago

Fixed in #188