[misc] Codacy fixes and more on README.

Signed-off-by: Xiangyu Bu <[email protected]>

README.md

@@ -298,7 +298,69 @@ Use command `onedrived-pref account add --code <some_code_here>`, where
 
 #### Adding Drives to `onedrived`
 
-TBA.
+After you authorize `onedrived` to access your OneDrive data, you are now able
+to add Drives. Each OneDrive account has one or more Drive associated, and
+`onedrived` allows you to choose which Drive to sync. Similar to the step of
+authorizing `onedrived`, the CLI provides both interactive mode and command
+mode.
+
+#### Interactive mode
+
+```bash
+$ onedrived-pref drive set
+Reading drives information from OneDrive server...
+
+All available Drives of authorized accounts:
+
+  #  Account Email    Drive ID          Type      Quota                        Status
+---  ---------------  ----------------  --------  ---------------------------  --------
+  0  <some_email>     <some_drive_id>   personal  5.3 GB Used / 33.0 GB Total  active
+
+Please enter row number of the Drive to add or modify (CTRL+C to abort): 0
+
+Going to add/edit Drive "<some_drive_id>" of account "<some_email>"...
+Enter the directory path to sync with this Drive [/home/xb/OneDrive]:  
+Syncing with directory "/home/xb/OneDrive"? [y/N]: y
+Enter the path to ignore file for this Drive [/home/xb/.config/onedrived/ignore_v2.txt]: 
+
+Successfully configured Drive <some_drive_id> of account <some_email> (<some_user_id>):
+  Local directory: /home/xb/OneDrive
+  Ignore file path: /home/xb/.config/onedrived/ignore_v2.txt
+```
+
+If you have more than one account authorized, all drives of all authorized
+accounts will appear in the table.
+
+#### Command mode
+
+Please find the available command-line arguments from help message using
+command `onedrived-pref drive set --help`. 
+
+### Set up webhook
+
+#### Webhook explained
+
+For now, refer to issue #19. More details TBA.
+
+#### Using `ngrok`-based webhook
+
+Download and install [ngrok](https://ngrok.com).
+
+By default, `onedrived` will look for `ngrok` binary from `PATH`. To specify
+path to the binary manually, set up environment variable `NGROK` when running
+`onedrived`. For example, `NGROK=~/utils/ngrok onedrived start --debug`.
+
+To use a custom config file for `ngrok`, set environment variable
+`NGROK_CONFIG_FILE` to path of your desired config file. Note that `onedrived`
+will create a HTTPS tunnel automatically and there is no need to specify
+tunnels. The purpose of using a custom `ngrok` config file should be to adjust
+resource usage, or link `ngrok` process with your paid `ngrok` account. The
+default `ngrok` config file shipped with `onedrived` turns off terminal output
+of `ngrok` and disables inspection database.
+
+#### Using direct connection
+
+TBA. Not applicable to most end-user machines.
 
 ### Run `onedrived` in debug mode
 
@@ -318,8 +380,8 @@ onedrived start --debug
 `onedrived` follows behavior of standard Python library function
 [`getproxies()`](https://docs.python.org/3/library/urllib.request.html#urllib.request.getproxies)
 to read proxies information from the OS. That is, run the command with
-environment variable `HTTP_PROXY` (or `http_proxy`) to set up a HTTP proxy, and variable
-`HTTPS_PROXY` (or `https_proxy`) to set up a HTTPS proxy. For example,
+environment variable `HTTP_PROXY` (or `http_proxy`) to set up a HTTP proxy, and
+variable `HTTPS_PROXY` (or `https_proxy`) to set up a HTTPS proxy. For example,
 
 ```bash
 $ HTTPS_PROXY=https://user:pass@host:port/some_path onedrived start --debug
@@ -335,8 +397,37 @@ A HTTPS proxy must have a verifiable SSL certificate.
 
 #### Edit configuration of an existing Drive
 
+#### Edit ignore list (selective sync)
+
 #### Remove a Drive from `onedrived`
 
+##### Interactive mode
+
+```bash
+$ onedrived-pref drive del
+Drives that have been set up:
+
+ #0 - Drive "<some_drive_id_here>":
+   Account:     <some_account_email> (<some_user_id_here>)
+   Local root:  /home/xb/OneDrive
+   Ignore file: /home/xb/.config/onedrived/ignore_v2.txt
+
+Please enter the # number of the Drive to delete (CTRL+C to abort): 0
+Continue to delete Drive "<some_drive_id_here>" (its local directory will NOT be deleted)? [y/N]: y
+Successfully deleted Drive "<some_drive_id_here>" from onedrived.
+```
+
+##### Command mode
+
+The command-mode equivalent is:
+
+```bash
+onedrived-pref drive del --drive-id <some_drive_id_here> [--yes]
+```
+
+If argument `--yes` is used, the specified Drive, if already added, will be
+deleted without confirmation. 
+
 #### Adjusting parameters of `onedrived`
 
 #### Check latest version of `onedrived`

onedrived/od_main.py

@@ -5,6 +5,7 @@
 import logging
 import os
 import signal
+import subprocess
 import sys
 import weakref
 
@@ -49,6 +50,21 @@ def shutdown_workers():
             w.join()
 
 
+def init_webhook():
+    global webhook_server, webhook_worker
+    try:
+        webhook_server = od_webhook.get_webhook_server(context)
+    except RuntimeError as e:
+        logging.critical('Error initializing webhook: %s', e)
+        raise SystemExit()
+    webhook_worker = od_webhook.WebhookWorkerThread(webhook_url=webhook_server.webhook_url,
+                                                    callback_func=repo_updated_callback,
+                                                    action_delay_sec=context.config['webhook_action_delay_sec'])
+    webhook_server.set_worker(webhook_worker)
+    webhook_worker.start()
+    webhook_server.start()
+
+
 def shutdown_webhook():
     if webhook_server:
         webhook_server.stop()
@@ -135,7 +151,8 @@ def delete_temp_files(all_accounts):
     logging.info('Sweeping onedrived temporary files from local repositories.')
     for repo in itertools.chain.from_iterable(all_accounts.values()):
         if os.path.isdir(repo.local_root):
-            os.system('find "%s" -type f -name "%s" -delete' % (repo.local_root, repo.path_filter.get_temp_name('*')))
+            subprocess.call(('find', repo.local_root, '-type', 'f',
+                             '-name',repo.path_filter.get_temp_name('*'), '-delete'))
 
 
 def repo_updated_callback(repo):
@@ -158,7 +175,7 @@ def repo_updated_callback(repo):
                    'workdir': os.getcwd()
                })
 def main():
-    global task_pool, webhook_server, webhook_worker
+    global webhook_server, webhook_worker
 
     # Exit program when receiving SIGTERM or SIGINT.
     signal.signal(signal.SIGTERM, shutdown_callback)
@@ -183,13 +200,7 @@ def main():
     init_task_pool_and_workers()
 
     # Start webhook.
-    webhook_server = od_webhook.get_webhook_server(context)
-    webhook_worker = od_webhook.WebhookWorkerThread(webhook_url=webhook_server.webhook_url,
-                                                    callback_func=repo_updated_callback,
-                                                    action_delay_sec=context.config['webhook_action_delay_sec'])
-    webhook_server.set_worker(webhook_worker)
-    webhook_worker.start()
-    webhook_server.start()
+    init_webhook()
 
     context.watcher = LocalRepositoryWatcher(task_pool=task_pool, loop=context.loop)
 

onedrived/od_models/dict_guard/__init__.py

@@ -16,6 +16,22 @@ def _test_bool_option(schema, opt_key):
     return opt_key in schema and schema[opt_key] is True
 
 
+def _test_str_subtype_file(key, value, schema):
+    if _test_bool_option(schema, 'to_abspath'):
+        value = os.path.abspath(value)
+    if not os.path.exists(value):
+        if _test_bool_option(schema, 'create_if_missing'):
+            with open(value, 'w'):
+                pass
+        else:
+            raise exceptions.PathDoesNotExist(key, value)
+    elif not os.path.isfile(value):
+        raise exceptions.PathIsNotFile(key, value)
+    elif 'permission' in schema:
+        with open(value, schema['permission']):
+            pass
+
+
 class GuardedDict:
 
     def __init__(self, config_dict, config_schema_dict):
@@ -33,24 +49,9 @@ def set_int(self, key, value, schema):
             raise exceptions.IntValueAboveMaximum(key, value, schema['maximum'])
         self.config_dict[key] = value
 
-    def _test_str_subtype_file(self, key, value, schema):
-        if _test_bool_option(schema, 'to_abspath'):
-            value = os.path.abspath(value)
-        if not os.path.exists(value):
-            if _test_bool_option(schema, 'create_if_missing'):
-                with open(value, 'w'):
-                    pass
-            else:
-                raise exceptions.PathDoesNotExist(key, value)
-        elif not os.path.isfile(value):
-            raise exceptions.PathIsNotFile(key, value)
-        elif 'permission' in schema:
-            with open(value, schema['permission']):
-                pass
-
     def set_str_subtype(self, key, value, schema):
         if schema['subtype'] == StringSubTypes.FILE:
-            self._test_str_subtype_file(key, value, schema)
+            _test_str_subtype_file(key, value, schema)
         else:
             raise exceptions.UnsupportedSchemaType(schema['subtype'], schema)
         self.config_dict[key] = value

onedrived/od_pref.py

@@ -212,7 +212,7 @@ def print_all_drives():
         authenticator, drives = od_auth.get_authenticator_and_drives(context, i)
         for d in drives:
             drive_objs.append(d)
-            drive_table.append((str(len(drive_table) - 1), profile.account_email,
+            drive_table.append((str(len(drive_table)), profile.account_email,
                                 d.id, d.drive_type, quota_short_str(d.quota), d.status.state))
         all_drives[i] = (profile, authenticator, drive_objs)
     click.secho(translator['od_pref.print_all_drives.all_drives_table.note'], bold=True)
@@ -247,8 +247,8 @@ def print_saved_drives():
 
 def index_to_drive_table_row(index, drive_table):
     if isinstance(index, int) and 0 <= index < len(drive_table):
-        email = drive_table[index + 1][1]  # Plus one to offset the header row.
-        drive_id = drive_table[index + 1][2]
+        email = drive_table[index][1]  # Plus one to offset the header row.
+        drive_id = drive_table[index][2]
         return email, drive_id
     raise ValueError('Index is not a valid row number.')
 

onedrived/od_repo.py

@@ -183,7 +183,7 @@ def update_item(self, item, parent_relpath, size_local=0, status=ItemRecordStatu
         else:
             raise ValueError('Unknown type of item "%s (%s)".' % (item.name, item.id))
         parent_reference = item.parent_reference
-        modified_time, modified_time_w = get_item_modified_datetime(item)
+        modified_time, _ = get_item_modified_datetime(item)
         modified_time_str = datetime_to_str(modified_time)
         created_time_str = datetime_to_str(get_item_created_datetime(item))
         with self._lock, self._conn:

onedrived/od_tasks/merge_dir.py

@@ -157,22 +157,15 @@ def _handle_remote_file_with_record(self, remote_item, item_record, item_stat, i
                     self.repo.delete_item(remote_item.name, self.rel_path, False)
             return self._handle_remote_file_without_record(remote_item, None, item_local_abspath, all_local_items)
 
-        remote_mtime, remote_mtime_w = get_item_modified_datetime(remote_item)
+        remote_mtime, _ = get_item_modified_datetime(remote_item)
         local_mtime_ts = item_stat.st_mtime if item_stat else None
         remote_mtime_ts = datetime_to_timestamp(remote_mtime)
         record_mtime_ts = datetime_to_timestamp(item_record.modified_time)
-        local_sha1_hash = None
         try:
             remote_sha1_hash = remote_item.file.hashes.sha1_hash
         except AttributeError:
             remote_sha1_hash = None
 
-        def get_local_sha1_hash():
-            nonlocal local_sha1_hash
-            if local_sha1_hash is None:
-                local_sha1_hash = sha1_value(item_local_abspath)
-            return local_sha1_hash
-
         if (remote_item.id == item_record.item_id and remote_item.c_tag == item_record.c_tag or
             remote_item.size == item_record.size and
                 diff_timestamps(remote_mtime_ts, record_mtime_ts) == 0):
@@ -185,7 +178,7 @@ def get_local_sha1_hash():
                     self.repo, self.task_pool, self.rel_path, remote_item.name, remote_item.id, False))
             elif (item_stat.st_size == item_record.size_local and
                   (diff_timestamps(local_mtime_ts, record_mtime_ts) == 0 or
-                   remote_sha1_hash and remote_sha1_hash == get_local_sha1_hash())):
+                   remote_sha1_hash and remote_sha1_hash == sha1_value(item_local_abspath))):
                 # If the local file matches the database record (i.e., same mtime timestamp or same content),
                 # simply return. This is the best case.
                 if diff_timestamps(local_mtime_ts, remote_mtime_ts) != 0:
@@ -217,7 +210,7 @@ def get_local_sha1_hash():
                     download_file.DownloadFileTask(self.repo, self.task_pool, remote_item, self.rel_path))
             elif item_stat.st_size == item_record.size_local and \
                     (diff_timestamps(local_mtime_ts, record_mtime_ts) == 0 or
-                     item_record.sha1_hash and item_record.sha1_hash == get_local_sha1_hash()):
+                     item_record.sha1_hash and item_record.sha1_hash == sha1_value(item_local_abspath)):
                 # Local file agrees with database record. This means that the remote file is strictly newer.
                 # The local file can be safely overwritten.
                 logging.debug('Local file "%s" agrees with db record but remote item is different. Overwrite local.',
@@ -228,7 +221,7 @@ def get_local_sha1_hash():
                 # So both the local file and remote file have been changed after the record was created.
                 equal_ts = diff_timestamps(local_mtime_ts, remote_mtime_ts) == 0
                 if (item_stat.st_size == remote_item.size and (
-                        (equal_ts or remote_sha1_hash and remote_sha1_hash == get_local_sha1_hash()))):
+                        (equal_ts or remote_sha1_hash and remote_sha1_hash == sha1_value(item_local_abspath)))):
                     # Fortunately the two files seem to be the same.
                     # Here the logic is written as if there is no size mismatch issue.
                     logging.debug(

onedrived/od_threads.py

@@ -25,6 +25,6 @@ def run(self):
                 break
             task = self.task_pool.pop_task()
             if task is not None:
-                logging.debug('Acquired task %s.', task)
+                logging.debug('Got task %s.', task)
                 task.handle()
         logging.debug('Stopped.')

onedrived/od_webhooks/ngrok_server.py

@@ -14,7 +14,7 @@ class WebhookConfig(http_server.WebhookConfig):
 
     def __init__(self, port=0, ngrok_config_path=None):
         super().__init__(port=port)
-        self.ngrok_path = os.getenv('NGROK_PATH', 'ngrok')
+        self.ngrok_path = os.getenv('NGROK', 'ngrok')
         ngrok_config_path = os.getenv('NGROK_CONFIG_FILE', ngrok_config_path)
         if isinstance(ngrok_config_path, str):
             self.ngrok_config_path = ngrok_config_path
@@ -76,7 +76,7 @@ def _find_ngrok_inspection_port(self):
         for c in psutil.Process(self.ngrok_proc.pid).connections():
             if c.laddr[0] == '127.0.0.1' and c.raddr == () and c.status == psutil.CONN_LISTEN:
                 return c.laddr
-        raise RuntimeError('Did not find inspection interface of ngrok.')
+        raise RuntimeError('Did not find API interface of ngrok.')
 
     def _read_ngrok_tunnels(self):
         webhook_urls = dict()