Skip to content

Commit

Permalink
Attribute error during timeout cancelling transfer
Browse files Browse the repository at this point in the history
- Change remaining instances of errors.TicketCancelTimeout to errors.TransferCancelTimeout
- Include an initial unit test that attempts to mimic two connection attempts:
  - Connection 1 initiates the transfer
  - Connection 2 issues the delete request
- Fixed unit test to incorporate suggestions:
  - Use testutil.create_tempfile to create temporary file
  - Use a smaller file, 8 MiB (hopefully) is enough to simulate the issue
  - Use monkeypatch.setattr() to modify global values for test_delete_conflict test
  - Use time.sleep() instead of reading one byte

Signed-off-by: jsattelb <jsattelb@ldeo.columbia.edu>
  • Loading branch information
jsattelb committed Jan 16, 2024
1 parent 4e5f7e0 commit 30c3f86
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
4 changes: 2 additions & 2 deletions ovirt_imageio/_internal/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,8 @@ def cancel(self, timeout=60):
True if ticket can be removed.
Raises:
errors.TicketCancelTimeout if timeout is non-zero and the ticket is
still used when the timeout expires.
errors.TransferCancelTimeout if timeout is non-zero and the ticket
is still used when the timeout expires.
"""
log.debug("Cancelling transfer %s", self.transfer_id)

Expand Down
2 changes: 1 addition & 1 deletion ovirt_imageio/_internal/handlers/tickets.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def delete(self, req, resp, ticket_id):
req.client_addr, ticket.transfer_id)
try:
self.auth.remove(ticket_id)
except errors.TicketCancelTimeout as e:
except errors.TransferCancelTimeout as e:
# The ticket is still used by some connection so we cannot
# remove it. The caller can retry the call again when the
# number connections reach zero.
Expand Down
22 changes: 22 additions & 0 deletions test/handlers/tickets_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import http.client as http_client
import json
import time
import uuid

import pytest
Expand Down Expand Up @@ -397,3 +398,24 @@ def test_delete_all(srv):
# Note: incorrect according to RFC, but required for vdsm.
assert res.getheader("content-length") == "0"
pytest.raises(KeyError, srv.auth.get, ticket["uuid"])


def test_delete_conflict(srv, tmpdir, monkeypatch):
image = testutil.create_tempfile(tmpdir, "image", size=8 * 1024**2)
ticket = testutil.create_ticket(url="file://" + str(image))
srv.auth.add(ticket)

# Use very short timeout to make the test fast.
monkeypatch.setattr(srv.config.control, "remove_timeout", 0.001)

with http.RemoteClient(srv.config) as remote_client:
remote_client.get("/images/" + ticket["uuid"])

# Wait enough time to get the server connnection thread block on the
# client socket trying to write more data.
time.sleep(0.1)

with http.ControlClient(srv.config) as control_client:
res = control_client.delete("/tickets/" + ticket["uuid"])
res.read()
assert res.status == 409

0 comments on commit 30c3f86

Please sign in to comment.