From 9f25ef8603790871c142b34efd5eabf0b5c2098d Mon Sep 17 00:00:00 2001 From: Salvatore Ingala <6681844+bigspider@users.noreply.github.com> Date: Thu, 10 Oct 2019 18:22:33 +0700 Subject: [PATCH] Fixes from PR review --- apps/cli/pisa-cli.py | 41 +++++++++++++++++-------------- pisa/api.py | 3 +-- pisa/logger.py | 5 +++- pisa/pisad.py | 4 +-- pisa/watcher.py | 1 + test/unit/test_api.py | 5 ---- test/unit/test_blob.py | 2 -- test/unit/test_block_processor.py | 1 - test/unit/test_cleaner.py | 1 - test/unit/test_encrypted_blob.py | 3 --- test/unit/test_inspector.py | 1 - test/unit/test_tools.py | 5 ++-- 12 files changed, 33 insertions(+), 39 deletions(-) diff --git a/apps/cli/pisa-cli.py b/apps/cli/pisa-cli.py index 6327214..8b181a1 100644 --- a/apps/cli/pisa-cli.py +++ b/apps/cli/pisa-cli.py @@ -2,7 +2,6 @@ import re import os import sys import json -import logging import requests from sys import argv from hashlib import sha256 @@ -10,11 +9,15 @@ from binascii import unhexlify from getopt import getopt, GetoptError from requests import ConnectTimeout, ConnectionError +from pisa.logger import Logger from apps.cli.blob import Blob from apps.cli.help import help_add_appointment, help_get_appointment from apps.cli import DEFAULT_PISA_API_SERVER, DEFAULT_PISA_API_PORT +logger = Logger("Client") + + # FIXME: TESTING ENDPOINT, WON'T BE THERE IN PRODUCTION def generate_dummy_appointment(): get_block_count_end_point = "http://{}:{}/get_block_count".format(pisa_api_server, pisa_api_port) @@ -49,14 +52,14 @@ def add_appointment(args): if os.path.isfile(fin): appointment_data = json.load(open(fin)) else: - logging.error("[Client] can't find file " + fin) + logger.error("Can't find file " + fin) else: - logging.error("[Client] no file provided as appointment. " + use_help) + logger.error("No file provided as appointment. " + use_help) else: appointment_data = json.loads(arg_opt) except json.JSONDecodeError: - logging.error("[Client] non-JSON encoded data provided as appointment. " + use_help) + logger.error("Non-JSON encoded data provided as appointment. " + use_help) if appointment_data: valid_locator = check_txid_format(appointment_data.get('tx_id')) @@ -67,22 +70,22 @@ def add_appointment(args): appointment_data.get('start_time'), appointment_data.get('end_time'), appointment_data.get('dispute_delta')) - logging.info("[Client] sending appointment to PISA") + logger.info("Sending appointment to PISA") try: r = requests.post(url=add_appointment_endpoint, json=json.dumps(appointment), timeout=5) - logging.info("[Client] {} (code: {}).".format(r.text, r.status_code)) + logger.info("{} (code: {}).".format(r.text, r.status_code)) except ConnectTimeout: - logging.error("[Client] can't connect to pisa API. Connection timeout.") + logger.error("Can't connect to pisa API. Connection timeout.") except ConnectionError: - logging.error("[Client] can't connect to pisa API. Server cannot be reached.") + logger.error("Can't connect to pisa API. Server cannot be reached.") else: - logging.error("[Client] the provided locator is not valid.") + logger.error("The provided locator is not valid.") else: - logging.error("[Client] no appointment data provided. " + use_help) + logger.error("No appointment data provided. " + use_help) def get_appointment(args): @@ -104,16 +107,16 @@ def get_appointment(args): print(json.dumps(r.json(), indent=4, sort_keys=True)) except ConnectTimeout: - logging.error("[Client] can't connect to pisa API. Connection timeout.") + logger.error("Can't connect to pisa API. Connection timeout.") except ConnectionError: - logging.error("[Client] can't connect to pisa API. Server cannot be reached.") + logger.error("Can't connect to pisa API. Server cannot be reached.") else: - logging.error("[Client] the provided locator is not valid.") + logger.error("The provided locator is not valid.") else: - logging.error("[Client] the provided locator is not valid.") + logger.error("The provided locator is not valid.") def build_appointment(tx, tx_id, start_block, end_block, dispute_delta): @@ -199,7 +202,7 @@ if __name__ == '__main__': sys.exit(help_get_appointment()) else: - logging.error("[Client] unknown command. Use help to check the list of available commands") + logger.error("Unknown command. Use help to check the list of available commands") else: sys.exit(show_usage()) @@ -210,14 +213,14 @@ if __name__ == '__main__': generate_dummy_appointment() else: - logging.error("[Client] unknown command. Use help to check the list of available commands") + logger.error("Unknown command. Use help to check the list of available commands") else: - logging.error("[Client] no command provided. Use help to check the list of available commands.") + logger.error("No command provided. Use help to check the list of available commands.") except GetoptError as e: - logging.error("[Client] {}".format(e)) + logger.error("{}".format(e)) except json.JSONDecodeError as e: - logging.error("[Client] non-JSON encoded appointment passed as parameter.") + logger.error("Non-JSON encoded appointment passed as parameter.") diff --git a/pisa/api.py b/pisa/api.py index 48b6c46..cd05e02 100644 --- a/pisa/api.py +++ b/pisa/api.py @@ -1,11 +1,10 @@ import json from flask import Flask, request, Response, abort, jsonify -from pisa import HOST, PORT, logging, bitcoin_cli +from pisa import HOST, PORT, logging from pisa.logger import Logger from pisa.watcher import Watcher from pisa.inspector import Inspector -from pisa import HOST, PORT, logging from pisa.appointment import Appointment from pisa.block_processor import BlockProcessor diff --git a/pisa/logger.py b/pisa/logger.py index 3dad0e0..95c4733 100644 --- a/pisa/logger.py +++ b/pisa/logger.py @@ -27,4 +27,7 @@ class Logger(object): logging.debug(StructuredMessage(self._add_prefix(msg), actor=self.actor, **kwargs)) def error(self, msg, **kwargs): - logging.error(StructuredMessage(self._add_prefix(msg), actor=self.actor, **kwargs)) \ No newline at end of file + logging.error(StructuredMessage(self._add_prefix(msg), actor=self.actor, **kwargs)) + + def warn(self, msg, **kwargs): + logging.warn(StructuredMessage(self._add_prefix(msg), actor=self.actor, **kwargs)) diff --git a/pisa/pisad.py b/pisa/pisad.py index 918ec76..152d7c9 100644 --- a/pisa/pisad.py +++ b/pisa/pisad.py @@ -5,7 +5,7 @@ from pisa.logger import Logger from pisa.api import start_api from pisa.tools import can_connect_to_bitcoind, in_correct_network -logger = Logger("Pisad") +logger = Logger("Daemon") if __name__ == '__main__': debug = False @@ -23,4 +23,4 @@ if __name__ == '__main__': logger.error("bitcoind is running on a different network, check conf.py and bitcoin.conf. Shutting down") else: - logger.error("can't connect to bitcoind. Shutting down") + logger.error("Can't connect to bitcoind. Shutting down") diff --git a/pisa/watcher.py b/pisa/watcher.py index 45ec563..5248b77 100644 --- a/pisa/watcher.py +++ b/pisa/watcher.py @@ -12,6 +12,7 @@ from pisa.utils.zmq_subscriber import ZMQHandler logger = Logger("Watcher") + class Watcher: def __init__(self, max_appointments=MAX_APPOINTMENTS): self.appointments = dict() diff --git a/test/unit/test_api.py b/test/unit/test_api.py index 8505bfa..5e4a6ee 100644 --- a/test/unit/test_api.py +++ b/test/unit/test_api.py @@ -183,8 +183,3 @@ def test_get_all_appointments_responder(): assert (set(responder_jobs) == set(local_locators)) assert (len(received_appointments["watcher_appointments"]) == 0) - - - - - diff --git a/test/unit/test_blob.py b/test/unit/test_blob.py index efd9e1a..7eb3418 100644 --- a/test/unit/test_blob.py +++ b/test/unit/test_blob.py @@ -87,5 +87,3 @@ def test_encrypt(): encrypted_blob2 = blob.encrypt(key) assert(encrypted_blob == encrypted_blob2 and id(encrypted_blob) != id(encrypted_blob2)) - - diff --git a/test/unit/test_block_processor.py b/test/unit/test_block_processor.py index b0a2bba..a57fbb0 100644 --- a/test/unit/test_block_processor.py +++ b/test/unit/test_block_processor.py @@ -73,4 +73,3 @@ def test_potential_matches_random_data(locator_uuid_map): # None of the txids should match assert len(potential_matches) == 0 - diff --git a/test/unit/test_cleaner.py b/test/unit/test_cleaner.py index 5206118..237a2e3 100644 --- a/test/unit/test_cleaner.py +++ b/test/unit/test_cleaner.py @@ -78,4 +78,3 @@ def test_delete_completed_jobs(): Cleaner.delete_completed_jobs(jobs, tx_job_map, completed_jobs, 0) assert not set(completed_jobs).issubset(jobs.keys()) - diff --git a/test/unit/test_encrypted_blob.py b/test/unit/test_encrypted_blob.py index 096c316..67270c2 100644 --- a/test/unit/test_encrypted_blob.py +++ b/test/unit/test_encrypted_blob.py @@ -34,6 +34,3 @@ def test_decrypt(): encrypted_blob = EncryptedBlob(encrypted_data) assert(encrypted_blob.decrypt(key) == data) - - - diff --git a/test/unit/test_inspector.py b/test/unit/test_inspector.py index 3aa68f6..551cfe3 100644 --- a/test/unit/test_inspector.py +++ b/test/unit/test_inspector.py @@ -230,4 +230,3 @@ def test_inspect(): and appointment.end_time == end_time and appointment.dispute_delta == dispute_delta and appointment.encrypted_blob.data == encrypted_blob and appointment.cipher == cipher and appointment.hash_function == hash_function) - diff --git a/test/unit/test_tools.py b/test/unit/test_tools.py index ba5b834..0e96b7c 100644 --- a/test/unit/test_tools.py +++ b/test/unit/test_tools.py @@ -8,9 +8,10 @@ def test_check_txid_format(): assert(check_txid_format(None) is False) assert(check_txid_format("") is False) assert(check_txid_format(0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef) is False) # wrong type - assert(check_txid_format("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef") is True) # lowercase - assert(check_txid_format("0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF") is True) # uppercase + assert(check_txid_format("abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcd") is True) # lowercase + assert(check_txid_format("ABCDEFABCDEFABCDEFABCDEFABCDEFABCDEFABCDEFABCDEFABCDEFABCDEFABCD") is True) # uppercase assert(check_txid_format("0123456789abcdef0123456789ABCDEF0123456789abcdef0123456789ABCDEF") is True) # mixed case + assert(check_txid_format("0123456789012345678901234567890123456789012345678901234567890123") is True) # only nums assert(check_txid_format("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdf") is False) # too short assert(check_txid_format("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0") is False) # too long assert(check_txid_format("g123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef") is False) # non-hex