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

CLI Extension #56

Merged
merged 28 commits into from
Oct 18, 2023
Merged

CLI Extension #56

merged 28 commits into from
Oct 18, 2023

Conversation

StefanGreve
Copy link
Member

@StefanGreve StefanGreve commented Oct 12, 2023

Description

This feature request serves as a collection of small TODOs related to the anonpy CLI.

Requirements

  • add a --gui flag and raise NotImplementedError("TODO")
  • add a --clip flag to the download and upload command that, when enabled, copies the URL to clipboard
  • add a --checksum flag for switching between MD5, SHA1 or SHA256
  • add a --provider flag for setting the target server, by using built-in providers implemented in the providers namespace
  • add a --algorithm flag to the CLI and settings file
  • update docs

Remarks

  • If the download or upload method are run with the --verbose flag, anonpy will automatically return the default checksum (MD5).
  • The default checksum should also be configurable via the ConfigHandler class (see also: ConfigHandler #13)

The following changes were made to the original specification:

  • instead of specifying a --provider flag, use the anonpy.ini file for configuring the requested API
  • use the --checksum flag for passing the expected checksum

@StefanGreve StefanGreve added the enhancement Improve an existing feature label Oct 12, 2023
@StefanGreve StefanGreve added this to the 1.0.0 milestone Oct 12, 2023
@StefanGreve StefanGreve self-assigned this Oct 12, 2023
@StefanGreve StefanGreve linked an issue Oct 12, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #56 (27f97da) into master (743247b) will decrease coverage by 2.27%.
The diff coverage is 43.33%.

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
- Coverage   75.75%   73.49%   -2.27%     
==========================================
  Files          18       19       +1     
  Lines         726      781      +55     
==========================================
+ Hits          550      574      +24     
- Misses        176      207      +31     
Flag Coverage Δ
unittests 73.49% <43.33%> (-2.27%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/anonpy/internals/request_handler.py 60.27% <100.00%> (ø)
src/anonpy/server_response.py 100.00% <100.00%> (ø)
src/anonpy/internals/config_handler.py 88.23% <60.00%> (-7.33%) ⬇️
src/anonpy/internals/log_handler.py 80.50% <37.50%> (-3.43%) ⬇️
src/anonpy/internals/utils.py 59.09% <55.55%> (+0.26%) ⬆️
src/anonpy/anonpy.py 32.78% <19.04%> (-7.69%) ⬇️

@StefanGreve StefanGreve changed the title Add --gui flag CLI Extension Oct 12, 2023
* use rich module for prettier console output
* add checksum flag
* implement print_diff helper function for comparing strings
* refactor __main__ code
* reserve single letter shortcuts for frequently used arguments
* add custom formatter class to parser to increase spacing in
  help command between long argument names and their description
* use console.print instead of raising exceptions for user-facing
  error messages
* make these methods more forgivable if a an option or section
  is not defined in ConfigHandler
@StefanGreve
Copy link
Member Author

StefanGreve commented Oct 14, 2023

Future TODOs:

  • rich is required by twine, which currently is a development dependency, so the CLI would likely crash in release builds
  • consider using the progress bar by rich in the AnonPy class as well (see also: download.py)
  • refactor download and upload method in AnonPy (pass size as argument in order to remove the second preview invocation)
  • document requirements (expected keys in server responses) for both the CLI and the core library

@StefanGreve StefanGreve added documentation Improvements or additions to documentation dependencies Changes related to dependency management labels Oct 14, 2023
* according to the rich docs, most applications only require a
  single console object
* implement truncate method in utils
* edit console messages in CLI
* fix typo in default download endpoint in config
* remove tqdm from prod dependencies
* output download path in download command in CLI
* rename endpoint parameter in RequestHandler methods to relative_path,
  as to correctly denote the relatvie path of a URL
* modify returned dictionary in AnonPy's download and upload method
  and make them more predictable
* refactor code in CLI
* rename init_settings method to init_config
* implement eval_config
* pass settings dictionary to commands instead of the config handler
  object

A note on terminology: in this context, we use the name config to
highlight the fact that the variable denotes a ConfigHandler object,
while settings is a combination of these config values and the passed
command line arguments (args).

The eval_config method was implemented to move this logic to a single
place, thus reducing the likelihood that sometime in the future an
error will be made here due to prior code repetition.
* move print_diff and copy_to_clipboard functions to utils namespace
* read hash algorithm from command line or config file

Note: I first thought about a string to class instance conversion in
order to pass the algorithm parameter to the checksum compute function,
but using a globals lookup was ugly, hence the str_to_hash_algorithm
function. I don't really like it, so perhaps it's better to change the
signature of the compute function to a string in future? Though this
switch case would still end up there, one way or the other...
@StefanGreve
Copy link
Member Author

Note: The default configured server (https://pixeldrain.com) is pretty quick with blocking bots after downloading the same file a few times

@StefanGreve StefanGreve marked this pull request as ready for review October 16, 2023 22:15
@StefanGreve StefanGreve merged commit 0d25eec into master Oct 18, 2023
10 checks passed
@StefanGreve StefanGreve deleted the 47_CliExtension branch October 18, 2023 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Changes related to dependency management documentation Improvements or additions to documentation enhancement Improve an existing feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

CLI Extension
1 participant