Skip to content
6 changes: 2 additions & 4 deletions irods/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,8 @@ class Ticket(Model):
write_file_limit = Column(Integer, "TICKET_WRITE_FILE_LIMIT", 2212)
write_byte_count = Column(Integer, "TICKET_WRITE_BYTE_COUNT", 2213)
write_byte_limit = Column(Integer, "TICKET_WRITE_BYTE_LIMIT", 2214)

## For now, use of these columns raises CAT_SQL_ERR in both PRC and iquest: (irods/irods#5929)
# create_time = Column(String, 'TICKET_CREATE_TIME', 2209)
# modify_time = Column(String, 'TICKET_MODIFY_TIME', 2210)
create_time = Column(DateTime, 'TICKET_CREATE_TIME', 2209, min_version=(4, 3, 0))
modify_time = Column(DateTime, 'TICKET_MODIFY_TIME', 2210, min_version=(4, 3, 0))

class DataObject(Model):
"""For queries of R_DATA_MAIN when joining to R_TICKET_MAIN.
Expand Down
60 changes: 57 additions & 3 deletions irods/test/ticket_test.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,25 @@
#! /usr/bin/env python

import calendar
import datetime
import logging
import os
import sys
import unittest
import time
import calendar
import unittest

import irods.test.helpers as helpers
import tempfile
from irods.session import iRODSSession
import irods.exception as ex
import irods.keywords as kw
from irods.ticket import Ticket
from irods.ticket import list_tickets, Ticket
from irods.models import TicketQuery, DataObject, Collection

Check failure on line 17 in irods/test/ticket_test.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff I001

I001: Import block is un-sorted or un-formatted [isort:unsorted-imports]


logger = logging.getLogger(__name__)


# As with most of the modules in this test suite, session objects created via
# make_session() are implicitly agents of a rodsadmin unless otherwise indicated.
# Counterexamples within this module shall be obvious as they are instantiated by
Expand All @@ -29,6 +34,17 @@
)


def delete_tickets(session, dry_run = False):
for res in session.query(TicketQuery.Ticket):
t = Ticket(session, result=res)
if dry_run in (False, None):
t.delete(**{kw.ADMIN_KW: ""})
elif isinstance(dry_run, list):
dry_run.append(t)
else:
logger.info('Found ticket: %s',t.string)

Check failure on line 45 in irods/test/ticket_test.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-format

Ruff format

Improper formatting


def delete_my_tickets(session):
my_userid = session.users.get(session.username).id
my_tickets = session.query(TicketQuery.Ticket).filter(
Expand Down Expand Up @@ -358,6 +374,30 @@
os.unlink(file_.name)
alice.cleanup()

def test_new_attributes_in_tickets__issue_801(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's explicitly mention the attributes being tested here (i.e. create_time and modify_time). These will not be "new" for very long.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw, id is also being tested!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, whatever attributes are relevant. Or, if you prefer, an absolute moment in time: "attributes_unavailable_before_irods_4_3" or something like that.


admin_ticket_for_bob = None

if (admin:=helpers.make_session()).server_version < (4, 3, 0):
self.skipTest('"create_time" and "modify_time" not supported for Ticket')
Comment on lines +382 to +383
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check isn't necessary since we no longer support anything earlier than 4.3.0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. we sure of that? I've heard concern from some on our team, in the not-so-distant past, that PRC may no longer be compatible with iRODS 3.0.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. we sure of that?

Yes. The 4.2 series is EOL which automatically makes iRODS 3 EOL.

For users running 4.2, they have the ability to install earlier versions of the PRC and/or compile Python 3 if PRC 3 is needed.

Of course, we don't make any guarantees about whether PRC 3 works with 4.2 deployments because that's not part of our testing.


try:
with self.login(self.bob) as bob:
bobs_ticket = Ticket(bob)
bobs_ticket.issue('write', helpers.home_collection(bob))
time.sleep(2)
bobs_ticket.modify('add', 'user', admin.username)
bobs_ticket = Ticket(bob, result=[], ticket=bobs_ticket.string)
self.assertGreaterEqual(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be assertGreater? Won't this test pass even if the modify_time did not change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's correct. We're comparing modify_time to a timestamp 1 second or more later than the create_time, which I think is reasonable from the sleep(2) and the user add. The only way it will pass is if the modify time falls in that window.

If modify_time did not change, then presumably it will be closer to create_time than 1 second in the future, so the test will fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see... so... we're comparing modify_time to create_time-plus-1. So it could be 1 or more seconds different. In that case... could we not have this?

self.assertGreater(bobs_ticket.modify_time, bobs_ticket.create_time)

bobs_ticket.modify_time,
bobs_ticket.create_time + datetime.timedelta(seconds=1)

Check failure on line 393 in irods/test/ticket_test.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-format

Ruff format

Improper formatting
)

admin_ticket_for_bob = Ticket(admin, result=[], ticket=bobs_ticket.string)
self.assertEqual(admin_ticket_for_bob.id, bobs_ticket.id)
finally:
if admin_ticket_for_bob:
admin_ticket_for_bob.delete(**{kw.ADMIN_KW:''})

class TestTicketOps(unittest.TestCase):

Expand Down Expand Up @@ -455,7 +495,21 @@
def test_coll_ticket_write(self):
self._ticket_write_helper(obj_type="coll")


def test_list_tickets__issue_120(self):

Check failure on line 499 in irods/test/ticket_test.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-format

Ruff format

Improper formatting

ses = self.sess

# t first assigned as a "utility" Ticket object
t = Ticket(ses).issue('read', helpers.home_collection(ses))

# This time, t receives attributes from an internal GenQuery result.
t = Ticket(ses, result=[], ticket=t.string)

# Check an id attribute is present and listed in the results from list_tickets
self.assertIn(t.id, (_.id for _ in list_tickets(ses)))


if __name__ == "__main__":
# let the tests find the parent irods lib
sys.path.insert(0, os.path.abspath("../.."))
Expand Down
62 changes: 60 additions & 2 deletions irods/ticket.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
from irods.api_number import api_number
from irods.message import iRODSMessage, TicketAdminRequest
from irods.models import TicketQuery
from irods.column import Like

Check failure on line 4 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff F401

F401: `irods.column.Like` imported but unused [Pyflakes:unused-import]
from collections.abc import Mapping, Sequence

import random
import string
import logging
import datetime
import calendar

Check failure on line 11 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff I001

I001: Import block is un-sorted or un-formatted [isort:unsorted-imports]


logger = logging.getLogger(__name__)
Expand All @@ -28,15 +30,71 @@
raise # final try at conversion, so a failure is an error


def list_tickets(session, *, raw=False, all=True):

Check failure on line 33 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff A002

A002: Function argument `all` is shadowing a Python builtin [flake8-builtins:builtin-argument-shadowing]
"""
Enumerates (via GenQuery1) all tickets visible by, or owned by, the current user.

Args:
session: An iRODSSession object for use in the query.
raw: True if only the queried rows are to be returned; False to construct Ticket objects for each row.
all: True if a comprehensive list is desired; otherwise only those
tickets owned by the calling user.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should update the implementation to always return the full listing of tickets and remove the all parameter.

Reading the iticket help text left me confused as to why there's a need for ls vs ls-all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll defer to the experts on this.


Returns:
An iterator over a range of ticket objects.
"""

Check failure on line 45 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff DOC202

DOC202: Docstring should not have a returns section because the function doesn't return anything [pydoclint:docstring-extraneous-returns]

Check failure on line 45 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff DOC402

DOC402: `yield` is not documented in docstring [pydoclint:docstring-missing-yields]

Check failure on line 45 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff D401

D401: First line of docstring should be in imperative mood: "Enumerates (via GenQuery1) all tickets visible by, or owned by, the current user." [pydocstyle:non-imperative-mood]
query = session.query(TicketQuery.Ticket)
if not all:
query = query.filter(
TicketQuery.Ticket.user_id == session.users.get(session.username).id
)

Check failure on line 50 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-format

Ruff format

Improper formatting
if raw:
yield from query
else:
yield from (Ticket(session, result=_) for _ in query)


class Ticket:
def __init__(self, session, ticket="", result=None, allow_punctuation=False):
self._session = session
try:
if result is not None:
if isinstance(result, Mapping):
if (single_string:=result.get(TicketQuery.Ticket.string, '')):

Check failure on line 62 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-format

Ruff format

Improper formatting
if ticket and (ticket != single_string):

Check failure on line 63 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff SIM102

SIM102: Use a single `if` statement instead of nested `if` statements [flake8-simplify:collapsible-if]

Check failure on line 63 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff SIM102

SIM102: Use a single `if` statement instead of nested `if` statements [flake8-simplify:collapsible-if]
raise RuntimeError(
f"The specified result contained a ticket string mismatched to the provided identifier ({ticket = })"

Check failure on line 65 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-check

Ruff E501

E501: Line too long (129 > 120) [pycodestyle:line-too-long]
)

# Allow limited query for the purpose of populating id and other attributes
if result == [] and ticket:
result[:] = list(session.query(TicketQuery.Ticket).filter(TicketQuery.Ticket.string == ticket))

if isinstance(result, Sequence):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have tests for all these different types of result being passed in?

if ticket:
result = [_ for _ in result if _[TicketQuery.Ticket.string] == ticket][:1]

if not result:
result = None
else:
result = result[0]

if result:
ticket = result[TicketQuery.Ticket.string]
for attr, value in TicketQuery.Ticket.__dict__.items():
if value is TicketQuery.Ticket.string: continue

Check failure on line 84 in irods/ticket.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-format

Ruff format

Improper formatting
if not attr.startswith("_"):
try:
setattr(self, attr, result[value])
except KeyError:
# backward compatibility with older schema versions
pass
except TypeError:
raise RuntimeError(
"If specified, 'result' parameter must be a TicketQuery.Ticket search result"
"If specified, 'result' parameter must be a TicketQuery.Ticket search result or iterable of same"
)
except IndexError:
raise RuntimeError(
"If both result and string are specified, at least one 'result' must match the ticket string"
)
self._ticket = (
ticket if ticket else self._generate(allow_punctuation=allow_punctuation)
Expand Down
Loading