From 485b471522968e1099cee3aa1e0055f01cd5dbec Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Tue, 8 Jan 2019 12:19:00 +0100 Subject: [PATCH] Fix some review comments --- .../NetworkedPrinterOutputDevice.py | 4 +- .../src/Cloud/CloudApiClient.py | 35 ++++++------ .../src/Cloud/CloudOutputDevice.py | 55 +++++-------------- .../src/Cloud/CloudOutputDeviceManager.py | 4 +- .../{CloudErrorObject.py => CloudError.py} | 2 +- .../src/Cloud/Translations.py | 31 +++++++++++ .../tests/Cloud/TestCloudApiClient.py | 4 +- 7 files changed, 73 insertions(+), 62 deletions(-) rename plugins/UM3NetworkPrinting/src/Cloud/Models/{CloudErrorObject.py => CloudError.py} (97%) create mode 100644 plugins/UM3NetworkPrinting/src/Cloud/Translations.py diff --git a/cura/PrinterOutput/NetworkedPrinterOutputDevice.py b/cura/PrinterOutput/NetworkedPrinterOutputDevice.py index 3dcc43dd00..4a8aa0a6b2 100644 --- a/cura/PrinterOutput/NetworkedPrinterOutputDevice.py +++ b/cura/PrinterOutput/NetworkedPrinterOutputDevice.py @@ -18,6 +18,7 @@ from enum import IntEnum import os # To get the username import gzip + class AuthState(IntEnum): NotAuthenticated = 1 AuthenticationRequested = 2 @@ -207,7 +208,8 @@ class NetworkedPrinterOutputDevice(PrinterOutputDevice): self._last_request_time = time() if not self._manager: - return Logger.log("e", "No network manager was created to execute the PUT call with.") + Logger.log("e", "No network manager was created to execute the PUT call with.") + return body = data if isinstance(data, bytes) else data.encode() # type: bytes reply = self._manager.put(request, body) diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudApiClient.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudApiClient.py index 302ca86d32..8f10d02802 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudApiClient.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudApiClient.py @@ -14,13 +14,17 @@ from cura.API import Account from .MeshUploader import MeshUploader from ..Models import BaseModel from .Models.CloudClusterResponse import CloudClusterResponse -from .Models.CloudErrorObject import CloudErrorObject +from .Models.CloudError import CloudError from .Models.CloudClusterStatus import CloudClusterStatus from .Models.CloudPrintJobUploadRequest import CloudPrintJobUploadRequest from .Models.CloudPrintResponse import CloudPrintResponse from .Models.CloudPrintJobResponse import CloudPrintJobResponse +## The generic type variable used to document the methods below. +CloudApiClientModel = TypeVar("Model", bound = BaseModel) + + ## The cloud API client is responsible for handling the requests and responses from the cloud. # Each method should only handle models instead of exposing Any HTTP details. class CloudApiClient: @@ -33,7 +37,7 @@ class CloudApiClient: ## Initializes a new cloud API client. # \param account: The user's account object # \param on_error: The callback to be called whenever we receive errors from the server. - def __init__(self, account: Account, on_error: Callable[[List[CloudErrorObject]], None]) -> None: + def __init__(self, account: Account, on_error: Callable[[List[CloudError]], None]) -> None: super().__init__() self._manager = QNetworkAccessManager() self._account = account @@ -115,33 +119,31 @@ class CloudApiClient: # Logger.log("i", "Received a reply %s from %s with %s", status_code, reply.url().toString(), response) return status_code, json.loads(response) except (UnicodeDecodeError, JSONDecodeError, ValueError) as err: - error = CloudErrorObject(code=type(err).__name__, title=str(err), http_code=str(status_code), - id=str(time()), http_status="500") + error = CloudError(code=type(err).__name__, title=str(err), http_code=str(status_code), + id=str(time()), http_status="500") Logger.logException("e", "Could not parse the stardust response: %s", error) return status_code, {"errors": [error.toDict()]} - ## The generic type variable used to document the methods below. - Model = TypeVar("Model", bound=BaseModel) - ## Parses the given models and calls the correct callback depending on the result. # \param response: The response from the server, after being converted to a dict. # \param on_finished: The callback in case the response is successful. # \param model_class: The type of the model to convert the response to. It may either be a single record or a list. def _parseModels(self, response: Dict[str, Any], - on_finished: Union[Callable[[Model], Any], Callable[[List[Model]], Any]], - model_class: Type[Model]) -> None: + on_finished: Union[Callable[[CloudApiClientModel], Any], + Callable[[List[CloudApiClientModel]], Any]], + model_class: Type[CloudApiClientModel]) -> None: if "data" in response: data = response["data"] if isinstance(data, list): - results = [model_class(**c) for c in data] # type: List[CloudApiClient.Model] - on_finished_list = cast(Callable[[List[CloudApiClient.Model]], Any], on_finished) + results = [model_class(**c) for c in data] # type: List[CloudApiClientModel] + on_finished_list = cast(Callable[[List[CloudApiClientModel]], Any], on_finished) on_finished_list(results) else: - result = model_class(**data) # type: CloudApiClient.Model - on_finished_item = cast(Callable[[CloudApiClient.Model], Any], on_finished) + result = model_class(**data) # type: CloudApiClientModel + on_finished_item = cast(Callable[[CloudApiClientModel], Any], on_finished) on_finished_item(result) elif "errors" in response: - self._on_error([CloudErrorObject(**error) for error in response["errors"]]) + self._on_error([CloudError(**error) for error in response["errors"]]) else: Logger.log("e", "Cannot find data or errors in the cloud response: %s", response) @@ -153,8 +155,9 @@ class CloudApiClient: # \param model: The type of the model to convert the response to. def _addCallback(self, reply: QNetworkReply, - on_finished: Union[Callable[[Model], Any], Callable[[List[Model]], Any]], - model: Type[Model], + on_finished: Union[Callable[[CloudApiClientModel], Any], + Callable[[List[CloudApiClientModel]], Any]], + model: Type[CloudApiClientModel], ) -> None: def parse() -> None: status_code, response = self._parseReply(reply) diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py index 093aa05ea9..e866303d27 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py @@ -7,7 +7,6 @@ from typing import Dict, List, Optional, Set, cast from PyQt5.QtCore import QObject, QUrl, pyqtProperty, pyqtSignal, pyqtSlot -from UM import i18nCatalog from UM.Backend.Backend import BackendState from UM.FileHandler.FileHandler import FileHandler from UM.Logger import Logger @@ -18,7 +17,8 @@ from cura.CuraApplication import CuraApplication from cura.PrinterOutput.NetworkedPrinterOutputDevice import AuthState, NetworkedPrinterOutputDevice from cura.PrinterOutput.PrinterOutputModel import PrinterOutputModel from cura.PrinterOutputDevice import ConnectionType -from plugins.UM3NetworkPrinting.src.Cloud.CloudOutputController import CloudOutputController + +from .CloudOutputController import CloudOutputController from ..MeshFormatHandler import MeshFormatHandler from ..UM3PrintJobOutputModel import UM3PrintJobOutputModel from .CloudProgressMessage import CloudProgressMessage @@ -30,37 +30,10 @@ from .Models.CloudPrintResponse import CloudPrintResponse from .Models.CloudPrintJobResponse import CloudPrintJobResponse from .Models.CloudClusterPrinterStatus import CloudClusterPrinterStatus from .Models.CloudClusterPrintJobStatus import CloudClusterPrintJobStatus +from .Translations import Translations from .Utils import findChanges, formatDateCompleted, formatTimeCompleted -## Class that contains all the translations for this module. -class T: - # The translation catalog for this device. - - _I18N_CATALOG = i18nCatalog("cura") - - PRINT_VIA_CLOUD_BUTTON = _I18N_CATALOG.i18nc("@action:button", "Print via Cloud") - PRINT_VIA_CLOUD_TOOLTIP = _I18N_CATALOG.i18nc("@properties:tooltip", "Print via Cloud") - - CONNECTED_VIA_CLOUD = _I18N_CATALOG.i18nc("@info:status", "Connected via Cloud") - BLOCKED_UPLOADING = _I18N_CATALOG.i18nc("@info:status", "Sending new jobs (temporarily) blocked, still sending " - "the previous print job.") - - COULD_NOT_EXPORT = _I18N_CATALOG.i18nc("@info:status", "Could not export print job.") - - ERROR = _I18N_CATALOG.i18nc("@info:title", "Error") - UPLOAD_ERROR = _I18N_CATALOG.i18nc("@info:text", "Could not upload the data to the printer.") - - UPLOAD_SUCCESS_TITLE = _I18N_CATALOG.i18nc("@info:title", "Data Sent") - UPLOAD_SUCCESS_TEXT = _I18N_CATALOG.i18nc("@info:status", "Print job was successfully sent to the printer.") - - JOB_COMPLETED_TITLE = _I18N_CATALOG.i18nc("@info:status", "Print finished") - JOB_COMPLETED_PRINTER = _I18N_CATALOG.i18nc("@info:status", - "Printer '{printer_name}' has finished printing '{job_name}'.") - - JOB_COMPLETED_NO_PRINTER = _I18N_CATALOG.i18nc("@info:status", "The print job '{job_name}' was finished.") - - ## The cloud output device is a network output device that works remotely but has limited functionality. # Currently it only supports viewing the printer and print job status and adding a new job to the queue. # As such, those methods have been implemented here. @@ -159,9 +132,9 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): def _setInterfaceElements(self) -> None: self.setPriority(2) # make sure we end up below the local networking and above 'save to file' self.setName(self._id) - self.setShortDescription(T.PRINT_VIA_CLOUD_BUTTON) - self.setDescription(T.PRINT_VIA_CLOUD_TOOLTIP) - self.setConnectionText(T.CONNECTED_VIA_CLOUD) + self.setShortDescription(Translations.PRINT_VIA_CLOUD_BUTTON) + self.setDescription(Translations.PRINT_VIA_CLOUD_TOOLTIP) + self.setConnectionText(Translations.CONNECTED_VIA_CLOUD) ## Called when Cura requests an output device to receive a (G-code) file. def requestWrite(self, nodes: List[SceneNode], file_name: Optional[str] = None, limit_mimetypes: bool = False, @@ -169,7 +142,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): # Show an error message if we're already sending a job. if self._progress.visible: - message = Message(text = T.BLOCKED_UPLOADING, title = T.ERROR, lifetime = 10) + message = Message(text = Translations.BLOCKED_UPLOADING, title = Translations.ERROR, lifetime = 10) message.show() return @@ -184,7 +157,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): mesh_format = MeshFormatHandler(file_handler, self.firmwareVersion) if not mesh_format.is_valid: Logger.log("e", "Missing file or mesh writer!") - return self._onUploadError(T.COULD_NOT_EXPORT) + return self._onUploadError(Translations.COULD_NOT_EXPORT) mesh = mesh_format.getBytes(nodes) @@ -292,9 +265,11 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): if job.state == "wait_cleanup" and job.key not in self._finished_jobs and job.owner == user_name: self._finished_jobs.add(job.key) Message( - title = T.JOB_COMPLETED_TITLE, - text = (T.JOB_COMPLETED_PRINTER.format(printer_name=job.assignedPrinter.name, job_name=job.name) - if job.assignedPrinter else T.JOB_COMPLETED_NO_PRINTER.format(job_name=job.name)), + title = Translations.JOB_COMPLETED_TITLE, + text = (Translations.JOB_COMPLETED_PRINTER.format(printer_name=job.assignedPrinter.name, + job_name=job.name) + if job.assignedPrinter else + Translations.JOB_COMPLETED_NO_PRINTER.format(job_name=job.name)), ).show() # Ensure UI gets updated @@ -330,7 +305,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): def _onUploadError(self, message = None) -> None: self._progress.hide() self._uploaded_print_job = None - Message(text = message or T.UPLOAD_ERROR, title = T.ERROR, lifetime = 10).show() + Message(text = message or Translations.UPLOAD_ERROR, title = Translations.ERROR, lifetime = 10).show() self.writeError.emit() ## Shows a message when the upload has succeeded @@ -338,7 +313,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): def _onPrintRequested(self, response: CloudPrintResponse) -> None: Logger.log("d", "The cluster will be printing this print job with the ID %s", response.cluster_job_id) self._progress.hide() - Message(text = T.UPLOAD_SUCCESS_TEXT, title = T.UPLOAD_SUCCESS_TITLE, lifetime = 5).show() + Message(text = Translations.UPLOAD_SUCCESS_TEXT, title = Translations.UPLOAD_SUCCESS_TITLE, lifetime = 5).show() self.writeFinished.emit() ## Gets the remote printers. diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py index b1dc13e34f..72ac34ff34 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py @@ -13,7 +13,7 @@ from cura.Settings.GlobalStack import GlobalStack from .CloudApiClient import CloudApiClient from .CloudOutputDevice import CloudOutputDevice from .Models.CloudClusterResponse import CloudClusterResponse -from .Models.CloudErrorObject import CloudErrorObject +from .Models.CloudError import CloudError from .Utils import findChanges @@ -138,7 +138,7 @@ class CloudOutputDeviceManager: ## Handles an API error received from the cloud. # \param errors: The errors received - def _onApiError(self, errors: List[CloudErrorObject]) -> None: + def _onApiError(self, errors: List[CloudError]) -> None: text = ". ".join(e.title for e in errors) # TODO: translate errors message = Message( text = text, diff --git a/plugins/UM3NetworkPrinting/src/Cloud/Models/CloudErrorObject.py b/plugins/UM3NetworkPrinting/src/Cloud/Models/CloudError.py similarity index 97% rename from plugins/UM3NetworkPrinting/src/Cloud/Models/CloudErrorObject.py rename to plugins/UM3NetworkPrinting/src/Cloud/Models/CloudError.py index 28b4d916a1..b53361022e 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/Models/CloudErrorObject.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/Models/CloudError.py @@ -7,7 +7,7 @@ from .BaseCloudModel import BaseCloudModel ## Class representing errors generated by the cloud servers, according to the JSON-API standard. # Spec: https://api-staging.ultimaker.com/connect/v1/spec -class CloudErrorObject(BaseCloudModel): +class CloudError(BaseCloudModel): ## Creates a new error object. # \param id: Unique identifier for this particular occurrence of the problem. # \param title: A short, human-readable summary of the problem that SHOULD NOT change from occurrence to occurrence diff --git a/plugins/UM3NetworkPrinting/src/Cloud/Translations.py b/plugins/UM3NetworkPrinting/src/Cloud/Translations.py new file mode 100644 index 0000000000..278bf91c37 --- /dev/null +++ b/plugins/UM3NetworkPrinting/src/Cloud/Translations.py @@ -0,0 +1,31 @@ +# Copyright (c) 2018 Ultimaker B.V. +# Cura is released under the terms of the LGPLv3 or higher. +from UM import i18nCatalog + + +## Class that contains all the translations for this module. +class Translations: + # The translation catalog for this device. + + _I18N_CATALOG = i18nCatalog("cura") + + PRINT_VIA_CLOUD_BUTTON = _I18N_CATALOG.i18nc("@action:button", "Print via Cloud") + PRINT_VIA_CLOUD_TOOLTIP = _I18N_CATALOG.i18nc("@properties:tooltip", "Print via Cloud") + + CONNECTED_VIA_CLOUD = _I18N_CATALOG.i18nc("@info:status", "Connected via Cloud") + BLOCKED_UPLOADING = _I18N_CATALOG.i18nc("@info:status", "Sending new jobs (temporarily) blocked, still sending " + "the previous print job.") + + COULD_NOT_EXPORT = _I18N_CATALOG.i18nc("@info:status", "Could not export print job.") + + ERROR = _I18N_CATALOG.i18nc("@info:title", "Error") + UPLOAD_ERROR = _I18N_CATALOG.i18nc("@info:text", "Could not upload the data to the printer.") + + UPLOAD_SUCCESS_TITLE = _I18N_CATALOG.i18nc("@info:title", "Data Sent") + UPLOAD_SUCCESS_TEXT = _I18N_CATALOG.i18nc("@info:status", "Print job was successfully sent to the printer.") + + JOB_COMPLETED_TITLE = _I18N_CATALOG.i18nc("@info:status", "Print finished") + JOB_COMPLETED_PRINTER = _I18N_CATALOG.i18nc("@info:status", + "Printer '{printer_name}' has finished printing '{job_name}'.") + + JOB_COMPLETED_NO_PRINTER = _I18N_CATALOG.i18nc("@info:status", "The print job '{job_name}' was finished.") \ No newline at end of file diff --git a/plugins/UM3NetworkPrinting/tests/Cloud/TestCloudApiClient.py b/plugins/UM3NetworkPrinting/tests/Cloud/TestCloudApiClient.py index 0c0c8cffdf..b57334b2da 100644 --- a/plugins/UM3NetworkPrinting/tests/Cloud/TestCloudApiClient.py +++ b/plugins/UM3NetworkPrinting/tests/Cloud/TestCloudApiClient.py @@ -12,7 +12,7 @@ from src.Cloud.Models.CloudClusterResponse import CloudClusterResponse from src.Cloud.Models.CloudClusterStatus import CloudClusterStatus from src.Cloud.Models.CloudPrintJobResponse import CloudPrintJobResponse from src.Cloud.Models.CloudPrintJobUploadRequest import CloudPrintJobUploadRequest -from src.Cloud.Models.CloudErrorObject import CloudErrorObject +from src.Cloud.Models.CloudError import CloudError from tests.Cloud.Fixtures import readFixture, parseFixture from .NetworkManagerMock import NetworkManagerMock @@ -20,7 +20,7 @@ from .NetworkManagerMock import NetworkManagerMock class TestCloudApiClient(TestCase): maxDiff = None - def _errorHandler(self, errors: List[CloudErrorObject]): + def _errorHandler(self, errors: List[CloudError]): raise Exception("Received unexpected error: {}".format(errors)) def setUp(self):