From 342aa12702dc4c7b6a997cdae8e6fcc176a38bf5 Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Fri, 6 Jun 2025 11:53:09 -0700
Subject: [PATCH 01/17] retrieve id from print_job

---
 printer/server.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/printer/server.py b/printer/server.py
index 3f07bd12..ec75515a 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -107,11 +107,22 @@ def send_file_to_printer(
     if args.development:
         logging.warning(f"server is in development mode, command would've been `{command}`")
     else:
+        print_id = ""
         print_job = subprocess.Popen(
             command,
             shell=True,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            text=True,
         )
         print_job.wait()
+        if print_job.returncode == 0: # success
+            print_id = print_job.stdout.read().strip().split(" ")[3]
+            logging.warning(print_id)
+        else: # failed
+            logging.error(f"printing failed with error: {print_job.stderr.read()}")
+
+
 
 
 @app.get("/healthcheck/printer")

From 881f8c0cf0d8607c4f1d7f0e4e260045b8452a2f Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Fri, 6 Jun 2025 11:56:02 -0700
Subject: [PATCH 02/17] from loggin.warning to logging.info

---
 printer/server.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/printer/server.py b/printer/server.py
index ec75515a..56d8aef5 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -118,7 +118,7 @@ def send_file_to_printer(
         print_job.wait()
         if print_job.returncode == 0: # success
             print_id = print_job.stdout.read().strip().split(" ")[3]
-            logging.warning(print_id)
+            logging.info(print_id)
         else: # failed
             logging.error(f"printing failed with error: {print_job.stderr.read()}")
 

From 420da309b885d54f146b78259df81d4d6e1dcbf9 Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Fri, 6 Jun 2025 11:53:09 -0700
Subject: [PATCH 03/17] retrieve id from print_job

---
 printer/server.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/printer/server.py b/printer/server.py
index 56d8aef5..3fe76792 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -107,6 +107,7 @@ def send_file_to_printer(
     if args.development:
         logging.warning(f"server is in development mode, command would've been `{command}`")
     else:
+        print_id = ""
         print_id = ""
         print_job = subprocess.Popen(
             command,
@@ -114,11 +115,14 @@ def send_file_to_printer(
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             text=True,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            text=True,
         )
         print_job.wait()
         if print_job.returncode == 0: # success
             print_id = print_job.stdout.read().strip().split(" ")[3]
-            logging.info(print_id)
+            logging.warning(print_id)
         else: # failed
             logging.error(f"printing failed with error: {print_job.stderr.read()}")
 

From 2b56f573cb490f0e67fafa59de66333afdbeb29b Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Fri, 6 Jun 2025 11:56:02 -0700
Subject: [PATCH 04/17] from loggin.warning to logging.info

---
 printer/server.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/printer/server.py b/printer/server.py
index 3fe76792..7128b4c3 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -122,7 +122,7 @@ def send_file_to_printer(
         print_job.wait()
         if print_job.returncode == 0: # success
             print_id = print_job.stdout.read().strip().split(" ")[3]
-            logging.warning(print_id)
+            logging.info(print_id)
         else: # failed
             logging.error(f"printing failed with error: {print_job.stderr.read()}")
 

From 991fdce7a4e670b9b36dc3a16e37ea0a73093ea3 Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Fri, 6 Jun 2025 11:53:09 -0700
Subject: [PATCH 05/17] retrieve id from print_job

---
 printer/server.py | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/printer/server.py b/printer/server.py
index 7128b4c3..3f07e99b 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -107,7 +107,6 @@ def send_file_to_printer(
     if args.development:
         logging.warning(f"server is in development mode, command would've been `{command}`")
     else:
-        print_id = ""
         print_id = ""
         print_job = subprocess.Popen(
             command,
@@ -115,14 +114,11 @@ def send_file_to_printer(
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             text=True,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
-            text=True,
         )
         print_job.wait()
         if print_job.returncode == 0: # success
             print_id = print_job.stdout.read().strip().split(" ")[3]
-            logging.info(print_id)
+            logging.info(f"extracted print id is {print_id}")
         else: # failed
             logging.error(f"printing failed with error: {print_job.stderr.read()}")
 

From 74be066ce4411a73c6e3d9aa11142dca416bc9cb Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Wed, 25 Jun 2025 20:46:14 -0700
Subject: [PATCH 06/17] added args to popen

---
 printer/test_server.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/printer/test_server.py b/printer/test_server.py
index 33d5e16c..ee36e4b2 100644
--- a/printer/test_server.py
+++ b/printer/test_server.py
@@ -1,9 +1,11 @@
 import importlib
 import io
 import os
+import subprocess
 import unittest
 from unittest import mock
 
+
 from fastapi.testclient import TestClient
 
 import server
@@ -57,6 +59,9 @@ def test_print_endpoint(self, mock_pathlib_unlink, mock_open_func, mock_popen, _
             mock.call(
                 "lp -n 1  -o sides=one-sided -o media=na_letter_8.5x11in -d HP_P2015_DN /tmp/test-id",
                 shell=True,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                text=True,
             ),
         )
 

From 188be8629a9287c56faab474344ed01251d1427b Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Wed, 25 Jun 2025 20:47:26 -0700
Subject: [PATCH 07/17] more fixes

---
 printer/test_server.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/printer/test_server.py b/printer/test_server.py
index ee36e4b2..110e00a8 100644
--- a/printer/test_server.py
+++ b/printer/test_server.py
@@ -99,6 +99,9 @@ def test_print_endpoint_dont_delete_pdf(
             mock.call(
                 "lp -n 1  -o sides=one-sided -o media=na_letter_8.5x11in -d HP_P2015_DN /tmp/test-id",
                 shell=True,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                text=True,
             ),
         )
 

From 1d6c256dc2ace2a33de8ea774483ace9244d9245 Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Wed, 25 Jun 2025 22:17:18 -0700
Subject: [PATCH 08/17] added unit tests for text-parsing

---
 printer/server.py      | 28 ++++++++++++++++++++++------
 printer/test_server.py | 29 +++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/printer/server.py b/printer/server.py
index 3f07e99b..3daa94a6 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -116,11 +116,18 @@ def send_file_to_printer(
             text=True,
         )
         print_job.wait()
+        print('print_job.returncode', print_job.returncode)
+        print('print_job.returncode', print_job.returncode)
         if print_job.returncode == 0: # success
-            print_id = print_job.stdout.read().strip().split(" ")[3]
-            logging.info(f"extracted print id is {print_id}")
+            try:
+                print_id = print_job.stdout.read().strip().split(" ")[3]
+                logging.info(print_id)
+            except Exception as e:
+                logging.error(f"failed to parse print job output!: {print_job.stdout.read()}")
+                return ""
         else: # failed
-            logging.error(f"printing failed with error: {print_job.stderr.read()}")
+            logging.error(f"printing page failed!: {print_job.stderr.read()}")
+            return 500
 
 
 
@@ -151,16 +158,25 @@ async def read_item(file: UploadFile = File(...), copies: str = Form(...), sides
         file_path = str(base / file_id)
         with open(file_path, "wb") as f:
             f.write(await file.read())
-        send_file_to_printer(
+        print_id = send_file_to_printer(
             str(file_path),
             copies,
             sides=sides,
         )
+
+        if print_id == 500:
+            return HTTPException(
+                status_code=500,
+                detail="printing failed, check logs",
+            )
+        result = {
+            "print_id": print_id,
+        }
         if args.dont_delete_pdfs:
           logging.info(f'--dont-delete-pdfs is set, skipping deletion of file {file_path}')
-          return "worked!"
+          return result
         pathlib.Path(file_path).unlink()
-        return "worked!"
+        return result
     except Exception:
         logging.exception("printing failed!")
         return HTTPException(
diff --git a/printer/test_server.py b/printer/test_server.py
index 110e00a8..2073b1ce 100644
--- a/printer/test_server.py
+++ b/printer/test_server.py
@@ -36,6 +36,13 @@ def test_health_check(self):
     def test_print_endpoint(self, mock_pathlib_unlink, mock_open_func, mock_popen, _):
         client = self.load_server_with_args()
         test_file = io.BytesIO(b"dummy file content")
+
+        mock_popen_result = mock.MagicMock()
+        mock_popen_result.returncode = 0
+        mock_popen_result.stdout.read.return_value = "request id is HP_LaserJet_p2015dn_Right-53 (1 file(s))"
+
+        mock_popen.return_value = mock_popen_result
+
         response = client.post(
             "/print",
             files={"file": ("test.txt", test_file, "text/plain")},
@@ -43,7 +50,10 @@ def test_print_endpoint(self, mock_pathlib_unlink, mock_open_func, mock_popen, _
         )
 
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.text, '"worked!"')
+        json_response = response.json()
+        self.assertEqual(json_response, {
+            "print_id": "HP_LaserJet_p2015dn_Right-53",
+        })
 
         mock_open_func.assert_called_once_with("/tmp/test-id", "wb")
 
@@ -74,6 +84,12 @@ def test_print_endpoint(self, mock_pathlib_unlink, mock_open_func, mock_popen, _
     def test_print_endpoint_dont_delete_pdf(
         self, mock_pathlib_unlink, mock_open_func, mock_popen, _
     ):
+        
+        mock_popen_result = mock.MagicMock()
+        mock_popen_result.returncode = 0
+        mock_popen_result.stdout.read.return_value = "request id is HP_LaserJet_p2015dn_Right-53 (1 file(s))"
+
+        mock_popen.return_value = mock_popen_result
         client = self.load_server_with_args(["--dont-delete-pdfs"])
         test_file = io.BytesIO(b"dummy file content")
         response = client.post(
@@ -83,7 +99,10 @@ def test_print_endpoint_dont_delete_pdf(
         )
 
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.text, '"worked!"')
+        json_response = response.json()
+        self.assertEqual(json_response, {
+            "print_id": "HP_LaserJet_p2015dn_Right-53",
+        })
 
         mock_open_func.assert_called_once_with("/tmp/test-id", "wb")
 
@@ -135,3 +154,9 @@ def test_print_endpoint_error(self, mock_pathlib_unlink, _, mock_popen):
 
 if __name__ == "__main__":
     unittest.main()
+
+
+# we need 2 more tests, what should the api do when the...
+# 1. statuscode of Popen is non zero, should probably return http 500
+# 2. statuscode of Popen is zero, but the lp command returns junk, like an empty string
+#    

From 936e0b14a7a15d72e9b69ec2030c205566a8a348 Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Wed, 25 Jun 2025 22:17:39 -0700
Subject: [PATCH 09/17] added changes for server.py

---
 printer/server.py | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/printer/server.py b/printer/server.py
index 3daa94a6..e94e3732 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -116,8 +116,9 @@ def send_file_to_printer(
             text=True,
         )
         print_job.wait()
+
         print('print_job.returncode', print_job.returncode)
-        print('print_job.returncode', print_job.returncode)
+
         if print_job.returncode == 0: # success
             try:
                 print_id = print_job.stdout.read().strip().split(" ")[3]
@@ -164,14 +165,9 @@ async def read_item(file: UploadFile = File(...), copies: str = Form(...), sides
             sides=sides,
         )
 
-        if print_id == 500:
-            return HTTPException(
-                status_code=500,
-                detail="printing failed, check logs",
-            )
-        result = {
-            "print_id": print_id,
-        }
+        if print_id == 500: raise Exception("printing failed with code 500")
+        
+        result = { "print_id": print_id, }
         if args.dont_delete_pdfs:
           logging.info(f'--dont-delete-pdfs is set, skipping deletion of file {file_path}')
           return result

From 3d428d4a526b145051e2b7a8a954aa72cfc0225d Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Thu, 26 Jun 2025 20:48:23 -0700
Subject: [PATCH 10/17] added unit tests for returncode 1 and 0

---
 printer/server.py      |  25 +++++-----
 printer/test_server.py | 103 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 115 insertions(+), 13 deletions(-)

diff --git a/printer/server.py b/printer/server.py
index e94e3732..b01b5e44 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -118,17 +118,18 @@ def send_file_to_printer(
         print_job.wait()
 
         print('print_job.returncode', print_job.returncode)
-
-        if print_job.returncode == 0: # success
-            try:
-                print_id = print_job.stdout.read().strip().split(" ")[3]
-                logging.info(print_id)
-            except Exception as e:
-                logging.error(f"failed to parse print job output!: {print_job.stdout.read()}")
-                return ""
-        else: # failed
+        if print_job.returncode != 0:
             logging.error(f"printing page failed!: {print_job.stderr.read()}")
-            return 500
+            return None
+        try:
+            print_id = print_job.stdout.read().strip().split(" ")[3]
+            logging.info(print_id)
+            return print_id
+        except Exception as e:
+            logging.error(f"failed to parse print job output!: {print_job.stdout.read()}")
+            return ""
+       
+            
 
 
 
@@ -165,9 +166,9 @@ async def read_item(file: UploadFile = File(...), copies: str = Form(...), sides
             sides=sides,
         )
 
-        if print_id == 500: raise Exception("printing failed with code 500")
+        if print_id == None: raise Exception("printing failed!")
         
-        result = { "print_id": print_id, }
+        result = { "print_id": print_id }
         if args.dont_delete_pdfs:
           logging.info(f'--dont-delete-pdfs is set, skipping deletion of file {file_path}')
           return result
diff --git a/printer/test_server.py b/printer/test_server.py
index 2073b1ce..a4e7dab3 100644
--- a/printer/test_server.py
+++ b/printer/test_server.py
@@ -129,7 +129,7 @@ def test_print_endpoint_dont_delete_pdf(
     @mock.patch("server.subprocess.Popen")
     @mock.patch("builtins.open", side_effect=FileNotFoundError("sorry!"))
     @mock.patch("pathlib.Path.unlink")
-    def test_print_endpoint_error(self, mock_pathlib_unlink, _, mock_popen):
+    def test_print_endpoint_file_not_found(self, mock_pathlib_unlink, _, mock_popen):
         client = self.load_server_with_args()
         test_file = io.BytesIO(b"dummy file content")
         response = client.post(
@@ -151,6 +151,107 @@ def test_print_endpoint_error(self, mock_pathlib_unlink, _, mock_popen):
         mock_popen.assert_not_called()
         mock_pathlib_unlink.assert_not_called()
 
+    @mock.patch("server.uuid.uuid4", return_value="test-id")
+    @mock.patch("server.subprocess.Popen")
+    @mock.patch("builtins.open", callable=mock.mock_open)
+    @mock.patch("pathlib.Path.unlink")
+    def test_print_endpoint_nonzero_returncode(self, mock_pathlib_unlink, mock_open_func, mock_popen, _):
+        client = self.load_server_with_args()
+        test_file = io.BytesIO(b"dummy file content")
+
+        mock_popen_result = mock.MagicMock()
+        mock_popen_result.returncode = 1
+        mock_popen.return_value = mock_popen_result
+
+        response = client.post(
+            "/print",
+            files={"file": ("test.txt", test_file, "text/plain")},
+            data={"copies": "1", "sides": "dark-side"},
+        )
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response.json(),
+            {
+                "status_code": 500,
+                "detail": "printing failed, check logs",
+                "headers": None,
+            },
+        )
+
+        mock_open_func.assert_called_once_with("/tmp/test-id", "wb")
+
+        mock_open_func().write.assert_called_once()
+        self.assertEqual(
+            mock_open_func().write.call_args_list[0], mock.call(b"dummy file content")
+        )
+
+        mock_popen.assert_called_once()
+
+        self.assertEqual(
+            mock_popen.call_args_list[0],
+            mock.call(
+                "lp -n 1  -o sides=dark-side -o media=na_letter_8.5x11in -d HP_P2015_DN /tmp/test-id",
+                shell=True,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                text=True,
+            ),
+        )
+
+        mock_pathlib_unlink.assert_called_once()
+
+    @mock.patch("server.uuid.uuid4", return_value="test-id")
+    @mock.patch("server.subprocess.Popen")
+    @mock.patch("builtins.open", callable=mock.mock_open)
+    @mock.patch("pathlib.Path.unlink")
+    def test_junk_print_id(self, mock_pathlib_unlink, mock_open_func, mock_popen, _):
+        client = self.load_server_with_args()
+        test_file = io.BytesIO(b"dummy file content")
+
+        mock_popen_result = mock.MagicMock()
+        mock_popen_result.returncode = 0
+        mock_popen_result.stdout.read.return_value = "junk output"
+        mock_popen.return_value = mock_popen_result
+
+        response = client.post(
+            "/print",
+            files={"file": ("test.txt", test_file, "text/plain")},
+            data={"copies": "1", "sides": "dark-side"},
+        )
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response.json(),
+            {
+                "print_id": "",
+            },
+        )
+
+        mock_open_func.assert_called_once_with("/tmp/test-id", "wb")
+
+        mock_open_func().write.assert_called_once()
+        self.assertEqual(
+            mock_open_func().write.call_args_list[0], mock.call(b"dummy file content")
+        )
+
+        mock_popen.assert_called_once()
+
+        self.assertEqual(
+            mock_popen.call_args_list[0],
+            mock.call(
+                "lp -n 1  -o sides=dark-side -o media=na_letter_8.5x11in -d HP_P2015_DN /tmp/test-id",
+                shell=True,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                text=True,
+            ),
+        )
+
+        mock_pathlib_unlink.assert_called_once()
+
+        
+
 
 if __name__ == "__main__":
     unittest.main()

From 32222b32768efafd73a066574a908594b311feb4 Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Thu, 26 Jun 2025 21:03:50 -0700
Subject: [PATCH 11/17] added better logging

---
 printer/server.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/printer/server.py b/printer/server.py
index b01b5e44..63b019fc 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -119,14 +119,14 @@ def send_file_to_printer(
 
         print('print_job.returncode', print_job.returncode)
         if print_job.returncode != 0:
-            logging.error(f"printing page failed!: {print_job.stderr.read()}")
+            logging.error(f"command returned code {print_job.returncode} stderr: {print_job.stderr.read()} stdout: {print_job.stdout.read()}")
             return None
         try:
             print_id = print_job.stdout.read().strip().split(" ")[3]
             logging.info(print_id)
             return print_id
-        except Exception as e:
-            logging.error(f"failed to parse print job output!: {print_job.stdout.read()}")
+        except Exception:
+            logging.exception(f"failed to extract print id from stdout: {print_job.stdout.read()}")
             return ""
        
             

From 2a184096adbf1189c4f406f3d6089ae7c2c3e28d Mon Sep 17 00:00:00 2001
From: epicgdog <91296067+epicgdog@users.noreply.github.com>
Date: Thu, 26 Jun 2025 21:25:22 -0700
Subject: [PATCH 12/17] fixed test typos

---
 printer/test_server.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/printer/test_server.py b/printer/test_server.py
index a4e7dab3..22967182 100644
--- a/printer/test_server.py
+++ b/printer/test_server.py
@@ -153,7 +153,7 @@ def test_print_endpoint_file_not_found(self, mock_pathlib_unlink, _, mock_popen)
 
     @mock.patch("server.uuid.uuid4", return_value="test-id")
     @mock.patch("server.subprocess.Popen")
-    @mock.patch("builtins.open", callable=mock.mock_open)
+    @mock.patch("builtins.open", new_callable=mock.mock_open)
     @mock.patch("pathlib.Path.unlink")
     def test_print_endpoint_nonzero_returncode(self, mock_pathlib_unlink, mock_open_func, mock_popen, _):
         client = self.load_server_with_args()
@@ -203,7 +203,7 @@ def test_print_endpoint_nonzero_returncode(self, mock_pathlib_unlink, mock_open_
 
     @mock.patch("server.uuid.uuid4", return_value="test-id")
     @mock.patch("server.subprocess.Popen")
-    @mock.patch("builtins.open", callable=mock.mock_open)
+    @mock.patch("builtins.open", new_callable=mock.mock_open)
     @mock.patch("pathlib.Path.unlink")
     def test_junk_print_id(self, mock_pathlib_unlink, mock_open_func, mock_popen, _):
         client = self.load_server_with_args()
@@ -217,7 +217,7 @@ def test_junk_print_id(self, mock_pathlib_unlink, mock_open_func, mock_popen, _)
         response = client.post(
             "/print",
             files={"file": ("test.txt", test_file, "text/plain")},
-            data={"copies": "1", "sides": "dark-side"},
+            data={"copies": "1", "sides": "one-sided"},
         )
 
         self.assertEqual(response.status_code, 200)
@@ -240,7 +240,7 @@ def test_junk_print_id(self, mock_pathlib_unlink, mock_open_func, mock_popen, _)
         self.assertEqual(
             mock_popen.call_args_list[0],
             mock.call(
-                "lp -n 1  -o sides=dark-side -o media=na_letter_8.5x11in -d HP_P2015_DN /tmp/test-id",
+                "lp -n 1  -o sides=one-sided -o media=na_letter_8.5x11in -d HP_P2015_DN /tmp/test-id",
                 shell=True,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,

From 27ef6af0fb570c7d8b64b16bdcd3dce0e92a68ae Mon Sep 17 00:00:00 2001
From: evan <evanuxd@gmail.com>
Date: Thu, 26 Jun 2025 21:44:25 -0700
Subject: [PATCH 13/17] tests are fixed

---
 printer/server.py      | 25 ++++++++++++-------------
 printer/test_server.py |  6 +-----
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/printer/server.py b/printer/server.py
index 63b019fc..7ee7aca3 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -26,7 +26,8 @@
     allow_headers=["*"],
 )
 logging.basicConfig(
-    format="%(asctime)s.%(msecs)03dZ %(processName)s %(threadName)s %(levelname)s:%(name)s:%(message)s",
+    # in mondo we trust
+    format="%(asctime)s.%(msecs)03dZ %(levelname)s:%(name)s:%(message)s",
     datefmt="%Y-%m-%dT%H:%M:%S",
     level=logging.INFO,
 )
@@ -117,7 +118,6 @@ def send_file_to_printer(
         )
         print_job.wait()
 
-        print('print_job.returncode', print_job.returncode)
         if print_job.returncode != 0:
             logging.error(f"command returned code {print_job.returncode} stderr: {print_job.stderr.read()} stdout: {print_job.stdout.read()}")
             return None
@@ -128,11 +128,12 @@ def send_file_to_printer(
         except Exception:
             logging.exception(f"failed to extract print id from stdout: {print_job.stdout.read()}")
             return ""
-       
-            
-
-
 
+def maybe_delete_pdf(file_path):
+    if args.dont_delete_pdfs:
+        logging.info(f'--dont-delete-pdfs is set, skipping deletion of file {file_path}')
+        return
+    pathlib.Path(file_path).unlink()
 
 @app.get("/healthcheck/printer")
 def api():
@@ -166,14 +167,12 @@ async def read_item(file: UploadFile = File(...), copies: str = Form(...), sides
             sides=sides,
         )
 
-        if print_id == None: raise Exception("printing failed!")
+        maybe_delete_pdf(file_path)
+
+        if not args.development and print_id == None:
+            raise Exception("printing failed!")
         
-        result = { "print_id": print_id }
-        if args.dont_delete_pdfs:
-          logging.info(f'--dont-delete-pdfs is set, skipping deletion of file {file_path}')
-          return result
-        pathlib.Path(file_path).unlink()
-        return result
+        return { "print_id": print_id }
     except Exception:
         logging.exception("printing failed!")
         return HTTPException(
diff --git a/printer/test_server.py b/printer/test_server.py
index 22967182..72de7527 100644
--- a/printer/test_server.py
+++ b/printer/test_server.py
@@ -172,11 +172,7 @@ def test_print_endpoint_nonzero_returncode(self, mock_pathlib_unlink, mock_open_
         self.assertEqual(response.status_code, 200)
         self.assertEqual(
             response.json(),
-            {
-                "status_code": 500,
-                "detail": "printing failed, check logs",
-                "headers": None,
-            },
+            {'status_code': 500, 'detail': 'printing failed, check logs', 'headers': None},
         )
 
         mock_open_func.assert_called_once_with("/tmp/test-id", "wb")

From 79097112ed26b64b13a4e2822396bde886ad6352 Mon Sep 17 00:00:00 2001
From: evan <evanuxd@gmail.com>
Date: Thu, 26 Jun 2025 21:45:02 -0700
Subject: [PATCH 14/17] lint

---
 printer/server.py      | 31 +++++++++++++++++++---------
 printer/test_server.py | 46 +++++++++++++++++++++++++-----------------
 2 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/printer/server.py b/printer/server.py
index 7ee7aca3..971d6759 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -61,9 +61,9 @@ def get_args() -> argparse.Namespace:
         "--dont-delete-pdfs",
         action="store_true",
         default=False,
-        help="specify if server should delete pdfs after printing"
+        help="specify if server should delete pdfs after printing",
     )
-    
+
     return parser.parse_args()
 
 
@@ -106,7 +106,9 @@ def send_file_to_printer(
     command = f"lp -n {num_copies} {maybe_page_range} -o sides={sides} -o media=na_letter_8.5x11in -d {PRINTER_NAME} {file_path}"
     metrics_handler.print_jobs_recieved.inc()
     if args.development:
-        logging.warning(f"server is in development mode, command would've been `{command}`")
+        logging.warning(
+            f"server is in development mode, command would've been `{command}`"
+        )
     else:
         print_id = ""
         print_job = subprocess.Popen(
@@ -119,34 +121,45 @@ def send_file_to_printer(
         print_job.wait()
 
         if print_job.returncode != 0:
-            logging.error(f"command returned code {print_job.returncode} stderr: {print_job.stderr.read()} stdout: {print_job.stdout.read()}")
+            logging.error(
+                f"command returned code {print_job.returncode} stderr: {print_job.stderr.read()} stdout: {print_job.stdout.read()}"
+            )
             return None
         try:
             print_id = print_job.stdout.read().strip().split(" ")[3]
             logging.info(print_id)
             return print_id
         except Exception:
-            logging.exception(f"failed to extract print id from stdout: {print_job.stdout.read()}")
+            logging.exception(
+                f"failed to extract print id from stdout: {print_job.stdout.read()}"
+            )
             return ""
 
+
 def maybe_delete_pdf(file_path):
     if args.dont_delete_pdfs:
-        logging.info(f'--dont-delete-pdfs is set, skipping deletion of file {file_path}')
+        logging.info(
+            f"--dont-delete-pdfs is set, skipping deletion of file {file_path}"
+        )
         return
     pathlib.Path(file_path).unlink()
 
+
 @app.get("/healthcheck/printer")
 def api():
     metrics_handler.last_health_check_request.set(int(time.time()))
     return "printer is up!"
 
+
 @app.get("/metrics", response_class=PlainTextResponse)
 def metrics():
     return prometheus_client.generate_latest()
 
 
 @app.post("/print")
-async def read_item(file: UploadFile = File(...), copies: str = Form(...), sides: str = Form(...)):
+async def read_item(
+    file: UploadFile = File(...), copies: str = Form(...), sides: str = Form(...)
+):
     """
     incoming request to print looks like
     {
@@ -171,8 +184,8 @@ async def read_item(file: UploadFile = File(...), copies: str = Form(...), sides
 
         if not args.development and print_id == None:
             raise Exception("printing failed!")
-        
-        return { "print_id": print_id }
+
+        return {"print_id": print_id}
     except Exception:
         logging.exception("printing failed!")
         return HTTPException(
diff --git a/printer/test_server.py b/printer/test_server.py
index 72de7527..dd651d74 100644
--- a/printer/test_server.py
+++ b/printer/test_server.py
@@ -39,7 +39,9 @@ def test_print_endpoint(self, mock_pathlib_unlink, mock_open_func, mock_popen, _
 
         mock_popen_result = mock.MagicMock()
         mock_popen_result.returncode = 0
-        mock_popen_result.stdout.read.return_value = "request id is HP_LaserJet_p2015dn_Right-53 (1 file(s))"
+        mock_popen_result.stdout.read.return_value = (
+            "request id is HP_LaserJet_p2015dn_Right-53 (1 file(s))"
+        )
 
         mock_popen.return_value = mock_popen_result
 
@@ -51,9 +53,12 @@ def test_print_endpoint(self, mock_pathlib_unlink, mock_open_func, mock_popen, _
 
         self.assertEqual(response.status_code, 200)
         json_response = response.json()
-        self.assertEqual(json_response, {
-            "print_id": "HP_LaserJet_p2015dn_Right-53",
-        })
+        self.assertEqual(
+            json_response,
+            {
+                "print_id": "HP_LaserJet_p2015dn_Right-53",
+            },
+        )
 
         mock_open_func.assert_called_once_with("/tmp/test-id", "wb")
 
@@ -84,10 +89,12 @@ def test_print_endpoint(self, mock_pathlib_unlink, mock_open_func, mock_popen, _
     def test_print_endpoint_dont_delete_pdf(
         self, mock_pathlib_unlink, mock_open_func, mock_popen, _
     ):
-        
+
         mock_popen_result = mock.MagicMock()
         mock_popen_result.returncode = 0
-        mock_popen_result.stdout.read.return_value = "request id is HP_LaserJet_p2015dn_Right-53 (1 file(s))"
+        mock_popen_result.stdout.read.return_value = (
+            "request id is HP_LaserJet_p2015dn_Right-53 (1 file(s))"
+        )
 
         mock_popen.return_value = mock_popen_result
         client = self.load_server_with_args(["--dont-delete-pdfs"])
@@ -100,9 +107,12 @@ def test_print_endpoint_dont_delete_pdf(
 
         self.assertEqual(response.status_code, 200)
         json_response = response.json()
-        self.assertEqual(json_response, {
-            "print_id": "HP_LaserJet_p2015dn_Right-53",
-        })
+        self.assertEqual(
+            json_response,
+            {
+                "print_id": "HP_LaserJet_p2015dn_Right-53",
+            },
+        )
 
         mock_open_func.assert_called_once_with("/tmp/test-id", "wb")
 
@@ -155,7 +165,9 @@ def test_print_endpoint_file_not_found(self, mock_pathlib_unlink, _, mock_popen)
     @mock.patch("server.subprocess.Popen")
     @mock.patch("builtins.open", new_callable=mock.mock_open)
     @mock.patch("pathlib.Path.unlink")
-    def test_print_endpoint_nonzero_returncode(self, mock_pathlib_unlink, mock_open_func, mock_popen, _):
+    def test_print_endpoint_nonzero_returncode(
+        self, mock_pathlib_unlink, mock_open_func, mock_popen, _
+    ):
         client = self.load_server_with_args()
         test_file = io.BytesIO(b"dummy file content")
 
@@ -172,7 +184,11 @@ def test_print_endpoint_nonzero_returncode(self, mock_pathlib_unlink, mock_open_
         self.assertEqual(response.status_code, 200)
         self.assertEqual(
             response.json(),
-            {'status_code': 500, 'detail': 'printing failed, check logs', 'headers': None},
+            {
+                "status_code": 500,
+                "detail": "printing failed, check logs",
+                "headers": None,
+            },
         )
 
         mock_open_func.assert_called_once_with("/tmp/test-id", "wb")
@@ -246,14 +262,6 @@ def test_junk_print_id(self, mock_pathlib_unlink, mock_open_func, mock_popen, _)
 
         mock_pathlib_unlink.assert_called_once()
 
-        
-
 
 if __name__ == "__main__":
     unittest.main()
-
-
-# we need 2 more tests, what should the api do when the...
-# 1. statuscode of Popen is non zero, should probably return http 500
-# 2. statuscode of Popen is zero, but the lp command returns junk, like an empty string
-#    

From 9659b18c27e20fc378f7dae40b8b1a682ed3ff2b Mon Sep 17 00:00:00 2001
From: evan <evanuxd@gmail.com>
Date: Thu, 26 Jun 2025 21:47:03 -0700
Subject: [PATCH 15/17] rebase

---
 printer/server.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/printer/server.py b/printer/server.py
index 971d6759..93cfdea3 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -127,7 +127,7 @@ def send_file_to_printer(
             return None
         try:
             print_id = print_job.stdout.read().strip().split(" ")[3]
-            logging.info(print_id)
+            logging.info(f"extracted print id is {print_id}")
             return print_id
         except Exception:
             logging.exception(

From 29be7a87a7929cb462941a29436b19c1b97923e1 Mon Sep 17 00:00:00 2001
From: evan <evanuxd@gmail.com>
Date: Thu, 26 Jun 2025 21:52:24 -0700
Subject: [PATCH 16/17] return early in if

---
 printer/server.py | 54 ++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/printer/server.py b/printer/server.py
index 93cfdea3..93809568 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -109,31 +109,33 @@ def send_file_to_printer(
         logging.warning(
             f"server is in development mode, command would've been `{command}`"
         )
-    else:
-        print_id = ""
-        print_job = subprocess.Popen(
-            command,
-            shell=True,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
-            text=True,
-        )
-        print_job.wait()
+        return None
+
+    print_job = subprocess.Popen(
+        command,
+        shell=True,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        text=True,
+    )
+    print_job.wait()
 
-        if print_job.returncode != 0:
-            logging.error(
-                f"command returned code {print_job.returncode} stderr: {print_job.stderr.read()} stdout: {print_job.stdout.read()}"
-            )
-            return None
-        try:
-            print_id = print_job.stdout.read().strip().split(" ")[3]
-            logging.info(f"extracted print id is {print_id}")
-            return print_id
-        except Exception:
-            logging.exception(
-                f"failed to extract print id from stdout: {print_job.stdout.read()}"
-            )
-            return ""
+    if print_job.returncode != 0:
+        logging.error(
+            f"command returned code {print_job.returncode} stderr: {print_job.stderr.read()} stdout: {print_job.stdout.read()}"
+        )
+        return None
+    try:
+        print_id = print_job.stdout.read().strip().split(" ")[3]
+        logging.info(f"extracted print id is {print_id}")
+        return print_id
+    except Exception:
+        logging.exception(
+            f"failed to extract print id from stdout: {print_job.stdout.read()}"
+        )
+        # need to find a better value to return when the command exited with code 0
+        # but the output could not be parsed for a job id.
+        return ''
 
 
 def maybe_delete_pdf(file_path):
@@ -182,9 +184,9 @@ async def read_item(
 
         maybe_delete_pdf(file_path)
 
-        if not args.development and print_id == None:
+        if not args.development and print_id is None:
             raise Exception("printing failed!")
-
+        print(":DDDDDD", print_id)
         return {"print_id": print_id}
     except Exception:
         logging.exception("printing failed!")

From c4b2899c0725992e87b1e0d15b75400765aeacc8 Mon Sep 17 00:00:00 2001
From: evan <evanuxd@gmail.com>
Date: Sat, 28 Jun 2025 17:45:44 -0700
Subject: [PATCH 17/17] better exception, comment for fun

---
 printer/server.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/printer/server.py b/printer/server.py
index 93809568..e71bc9de 100644
--- a/printer/server.py
+++ b/printer/server.py
@@ -133,8 +133,8 @@ def send_file_to_printer(
         logging.exception(
             f"failed to extract print id from stdout: {print_job.stdout.read()}"
         )
-        # need to find a better value to return when the command exited with code 0
-        # but the output could not be parsed for a job id.
+        # need to find a better value to return when the command exited
+        # with code 0 but the output could not be parsed for a job id.
         return ''
 
 
@@ -185,8 +185,7 @@ async def read_item(
         maybe_delete_pdf(file_path)
 
         if not args.development and print_id is None:
-            raise Exception("printing failed!")
-        print(":DDDDDD", print_id)
+            raise Exception("unable to extract print id from print request")
         return {"print_id": print_id}
     except Exception:
         logging.exception("printing failed!")