Fix some review comments

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)

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)

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.

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,

plugins/UM3NetworkPrinting/src/Cloud/Models/CloudErrorObject.py → 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

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.")

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):