Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: add .to_dict() method to PrioritizedISBN for serialization #8910

Conversation

scottbarnes
Copy link
Collaborator

@scottbarnes scottbarnes commented Mar 15, 2024

When checking /status on the affiliate-server, if there is an item in the queue, rather than display the count, the server would respond with a rather verbose error because to display items in the queue, they must be first serialized via json.dumps, but prior to this commit, the PrioritizedISBN class could not be serialized, and the queue contains only PrioritizedISBN items.

For reference, here is what would happen:

172.28.0.1:59064 - - [15/Mar/2024 22:20:29] "HTTP/1.1 GET /status" - 500 Internal Server Error
Traceback (most recent call last):
  File "/home/openlibrary/.local/lib/python3.12/site-packages/web/application.py", line 276, in process
    return self.handle()
           ^^^^^^^^^^^^^
  File "/home/openlibrary/.local/lib/python3.12/site-packages/web/application.py", line 267, in handle
    return self._delegate(fn, self.fvars, args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/openlibrary/.local/lib/python3.12/site-packages/web/application.py", line 509, in _delegate
    return handle_class(cls)
           ^^^^^^^^^^^^^^^^^
  File "/home/openlibrary/.local/lib/python3.12/site-packages/web/application.py", line 487, in handle_class
    return tocall(*args)
           ^^^^^^^^^^^^^
  File "/openlibrary/./scripts/affiliate_server.py", line 305, in GET
    return json.dumps(
           ^^^^^^^^^^^
  File "/usr/local/lib/python3.12/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/json/encoder.py", line 200, in encode
    chunks = self.iterencode(o, _one_shot=True)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/json/encoder.py", line 258, in iterencode
    return _iterencode(o, 0)
           ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/json/encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type PrioritizedISBN is not JSON serializable

Now, as the accompanying tests show, they can be serialized with the to_dict() method.

Testing

It can be hard to 'catch' items in the queue, since it's processing up to 10 per second; to test this I made 30 concurrent requests to the local affiliate server and quickly checked the queue, catching 8 items in there:

curl localhost:31337/status; echo "";             
{"thread_is_alive": true, "queue_size": 8, "queue": [{"isbn": "816785146X", "priority": "LOW", "timestamp": "2024-03-15T23:05:30.601706"}, {"isbn": "6755282065", "priority": "LOW", "timestamp": "2024-03-15T23:05:30.605694"}, {"isbn": "4906172431", "priority": "LOW", "timestamp": "202
4-03-15T23:05:30.604042"}, {"isbn": "0170868575", "priority": "LOW", "timestamp": "2024-03-15T23:05:30.605997"}, {"isbn": "0567433013", "priority": "LOW", "timestamp": "2024-03-15T23:05:30.606594"}, {"isbn": "8803177922", "priority": "LOW", "timestamp": "2024-03-15T23:05:30.605098"},
 {"isbn": "9945991396", "priority": "LOW", "timestamp": "2024-03-15T23:05:30.605412"}, {"isbn": "7023385029", "priority": "LOW", "timestamp": "2024-03-15T23:05:30.796779"}]}

Screenshot

Stakeholders

@mekarpeles

When checking `/status` on the affiliate-server, *if* there is an item
in the queue, the server would respond with an error because to display
items in the queue, they must be first serialized via `json.dumps`, but
prior to this commit, the `PrioritizedISBN` class could not be
serialized.

For reference, here is what would happen:
```
172.28.0.1:59064 - - [15/Mar/2024 22:20:29] "HTTP/1.1 GET /status" - 500 Internal Server Error
Traceback (most recent call last):
  File "/home/openlibrary/.local/lib/python3.12/site-packages/web/application.py", line 276, in process
    return self.handle()
           ^^^^^^^^^^^^^
  File "/home/openlibrary/.local/lib/python3.12/site-packages/web/application.py", line 267, in handle
    return self._delegate(fn, self.fvars, args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/openlibrary/.local/lib/python3.12/site-packages/web/application.py", line 509, in _delegate
    return handle_class(cls)
           ^^^^^^^^^^^^^^^^^
  File "/home/openlibrary/.local/lib/python3.12/site-packages/web/application.py", line 487, in handle_class
    return tocall(*args)
           ^^^^^^^^^^^^^
  File "/openlibrary/./scripts/affiliate_server.py", line 305, in GET
    return json.dumps(
           ^^^^^^^^^^^
  File "/usr/local/lib/python3.12/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/json/encoder.py", line 200, in encode
    chunks = self.iterencode(o, _one_shot=True)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/json/encoder.py", line 258, in iterencode
    return _iterencode(o, 0)
           ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/json/encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type PrioritizedISBN is not JSON serializable
```

Now, as the accompany tests show, they can be serialized with the
`to_dict()` method.
@mekarpeles
Copy link
Member

Very good catch, thank you @scottbarnes !

@mekarpeles mekarpeles merged commit 7f69396 into internetarchive:master Mar 18, 2024
3 checks passed
@mekarpeles mekarpeles self-assigned this Mar 18, 2024
@scottbarnes scottbarnes deleted the fix/allow-priortizedisbn-to-serialize branch March 18, 2024 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants