Skip to content

Commit 45e8ff0

Browse files
d-w-mooretrel
authored andcommitted
[#455] introduce low level ticket api changes
This ensures the application of a ticket to a session via supply() filters down to each connection instantiated for that session object. One test (test_object_read_and_write_tickets) was updated to ensure call to cleanup() on a session, in order to clear out existing connections (with their associated tickets) when required. Due to the flawed approach previously used in the ticket API, this step had not been strictly necessary for the test to pass.
1 parent 3329b64 commit 45e8ff0

5 files changed

Lines changed: 64 additions & 28 deletions

File tree

irods/manager/data_object_manager.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def should_parallelize_transfer( self,
6464
# Example: $ export IRODS_VERSION_OVERRIDE="4,2,9" ; python -m irods.parallel ...
6565
# ---
6666
# Delete the following line on resolution of https://github.com/irods/irods/issues/5932 :
67-
if getattr(self.sess,'ticket__',None) is not None: return False
67+
if self.sess.ticket__: return False
6868
server_version = ( ast.literal_eval(os.environ.get('IRODS_VERSION_OVERRIDE', '()' )) or server_version_hint or
6969
self.server_version )
7070
if num_threads == 1 or ( server_version < parallel.MINIMUM_SERVER_VERSION ):
@@ -126,7 +126,7 @@ def get(self, path, local_path = None, num_threads = DEFAULT_NUMBER_OF_THREADS,
126126
.filter(DataObject.collection_id == parent.id)\
127127
.add_keyword(kw.ZONE_KW, path.split('/')[1])
128128

129-
if hasattr(self.sess,'ticket__'):
129+
if self.sess.ticket__:
130130
query = query.filter(Collection.id != 0) # a no-op, but necessary because CAT_SQL_ERR results if the ticket
131131
# is for a DataObject and we don't explicitly join to Collection
132132

irods/pool.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
import logging
44
import threading
55
import os
6+
import weakref
67

78
from irods import DEFAULT_CONNECTION_TIMEOUT
89
from irods.connection import Connection
10+
from irods.ticket import Ticket
911

1012
logger = logging.getLogger(__name__)
1113

@@ -22,13 +24,14 @@ def method_(self,*s,**kw):
2224

2325
class Pool(object):
2426

25-
def __init__(self, account, application_name='', connection_refresh_time=-1):
27+
def __init__(self, account, application_name='', connection_refresh_time=-1, session = None):
2628
'''
2729
Pool( account , application_name='' )
2830
Create an iRODS connection pool; 'account' is an irods.account.iRODSAccount instance and
2931
'application_name' specifies the application name as it should appear in an 'ips' listing.
3032
'''
3133

34+
self.session_ref = weakref.ref(session) if session is not None else lambda:None
3235
self._thread_local = threading.local()
3336
self.account = account
3437
self._lock = threading.RLock()
@@ -77,6 +80,11 @@ def get_connection(self):
7780

7881
self.active.add(conn)
7982

83+
sess = self.session_ref()
84+
if sess and sess.ticket__ and not sess.ticket_applied.get(conn,False):
85+
Ticket._lowlevel_api_request(conn, "session", sess.ticket__)
86+
sess.ticket_applied[conn] = True
87+
8088
logger.debug("Adding connection with id {} to active set".format(id(conn)))
8189

8290
logger.debug('num active: {}'.format(len(self.active)))
@@ -100,3 +108,4 @@ def release_connection(self, conn, destroy=False):
100108
self.idle.remove(conn)
101109
logger.debug('num active: {}'.format(len(self.active)))
102110
logger.debug('num idle: {}'.format(len(self.idle)))
111+

irods/session.py

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import absolute_import
22
import atexit
3+
import copy
34
import os
45
import ast
56
import json
@@ -145,7 +146,9 @@ def __init__(self, configure = True, auto_cleanup = True, **kwargs):
145146
self.user_groups = GroupManager(self)
146147
self.resources = ResourceManager(self)
147148
self.zones = ZoneManager(self)
148-
149+
self._auto_cleanup = auto_cleanup
150+
self.ticket__ = ''
151+
self.ticket_applied = weakref.WeakKeyDictionary() # conn -> ticket applied
149152
if auto_cleanup:
150153
_weakly_reference(self)
151154

@@ -162,14 +165,33 @@ def __del__(self):
162165
if self.pool is not None:
163166
self.cleanup()
164167

165-
def cleanup(self):
166-
for conn in self.pool.active | self.pool.idle:
167-
try:
168-
conn.disconnect()
169-
except NetworkException:
170-
pass
171-
conn.release(True)
168+
def clone(self, **kwargs):
169+
other = copy.copy(self)
170+
other.pool = None
171+
for k,v in vars(other).items():
172+
setter = getattr(v,'_set_manager_session',None)
173+
if setter:
174+
setter(other)
175+
other.cleanup(new_host = kwargs.pop('host',''))
176+
other.ticket__ = kwargs.pop('ticket',self.ticket__)
177+
self.ticket_applied = weakref.WeakKeyDictionary() # conn -> ticket applied
178+
if other._auto_cleanup:
179+
_weakly_reference(other)
180+
return other
181+
182+
def cleanup(self, new_host = ''):
183+
if self.pool:
184+
for conn in self.pool.active | self.pool.idle:
185+
try:
186+
conn.disconnect()
187+
except NetworkException:
188+
pass
189+
conn.release(True)
172190
if self.do_configure:
191+
if new_host:
192+
d = self.do_configure.setdefault('_overrides',{})
193+
d['irods_host'] = new_host
194+
self.__configured = None
173195
self.__configured = self.configure(**self.do_configure)
174196

175197
def _configure_account(self, **kwargs):

irods/test/ticket_test.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,10 @@ def test_ticket_expiry (self):
163163
def test_object_read_and_write_tickets(self):
164164
if self.alice is None or self.bob is None:
165165
self.skipTest("A rodsuser (alice and/or bob) could not be created.")
166-
t=None
167-
data_objs=[]
168-
tmpfiles=[]
166+
t = None
167+
data_objs = []
168+
tmpfiles = []
169+
tickets = {}
169170
try:
170171
# Create ticket for read access to alice's home collection.
171172
alice = self.login(self.alice)
@@ -174,14 +175,14 @@ def test_object_read_and_write_tickets(self):
174175
# Create 'R' and 'W' in alice's home collection.
175176
data_objs = [helpers.make_object(alice,home.path+"/"+name,content='abcxyz') for name in ('R','W')]
176177
tickets = {
177-
'R': Ticket(alice).issue('read', home.path + "/R").string,
178-
'W': Ticket(alice).issue('write', home.path + "/W").string
178+
'R': Ticket(alice).issue('read', home.path + "/R"),
179+
'W': Ticket(alice).issue('write', home.path + "/W")
179180
}
180181
# Test only write ticket allows upload.
181182
with self.login(self.bob) as bob:
182183
rw_names={}
183184
for name in ('R','W'):
184-
Ticket( bob, tickets[name] ).supply()
185+
Ticket( bob, tickets[name].string ).supply()
185186
with tempfile.NamedTemporaryFile (delete=False) as tmpf:
186187
tmpfiles += [tmpf]
187188
rw_names[name] = tmpf.name
@@ -197,16 +198,17 @@ def test_object_read_and_write_tickets(self):
197198
raise AssertionError("A read ticket allowed a data object write operation to happen without error.")
198199

199200
# Test upload was successful, by getting and confirming contents.
200-
201201
with self.login(self.bob) as bob: # This check must be in a new session or we get CollectionDoesNotExist. - Possibly a new issue [ ]
202202
for name in ('R','W'):
203-
Ticket( bob, tickets[name] ).supply()
203+
bob.cleanup() # clear out existing connections
204+
Ticket( bob, tickets[name].string ).supply()
204205
bob.data_objects.get(home.path+"/"+name,rw_names[ name ],**{kw.FORCE_FLAG_KW:''})
205206
with open(rw_names[ name ],'r') as tmpread:
206207
self.assertEqual(tmpread.read(),
207208
'abcxyz' if name == 'R' else 'hello')
208209
finally:
209-
if t: t.delete()
210+
for t in tickets.values():
211+
t.delete()
210212
for d in data_objs:
211213
d.unlink(force=True)
212214
for file_ in tmpfiles: os.unlink( file_.name )

irods/ticket.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,19 @@ def _generate(self, length=15, allow_punctuation = False):
5757
source_characters += string.punctuation
5858
return ''.join(random.SystemRandom().choice(source_characters) for _ in range(length))
5959

60-
def _api_request(self,cmd_string,*args, **opts):
61-
message_body = TicketAdminRequest(cmd_string, self.ticket, *args, **opts)
62-
message = iRODSMessage("RODS_API_REQ", msg=message_body, int_info=api_number['TICKET_ADMIN_AN'])
63-
60+
def _api_request(self, cmd_string, *args, **opts):
6461
with self.session.pool.get_connection() as conn:
65-
conn.send(message)
66-
response = conn.recv()
62+
self._lowlevel_api_request(conn, cmd_string, self.ticket, *args, **opts)
6763
return self
6864

65+
@staticmethod
66+
def _lowlevel_api_request(conn_, cmd_string, ticket_string, *args, **opts):
67+
message_body = TicketAdminRequest(cmd_string, ticket_string, *args, **opts)
68+
message = iRODSMessage("RODS_API_REQ", msg=message_body, int_info=api_number['TICKET_ADMIN_AN'])
69+
conn_.send(message)
70+
response = conn_.recv()
71+
return response
72+
6973
def issue(self,permission,target,**opt): return self._api_request("create",permission,target,**opt)
7074

7175
create = issue
@@ -77,9 +81,8 @@ def modify(self,*args,**opt):
7781
return self._api_request("mod",*arglist,**opt)
7882

7983
def supply(self,**opt):
80-
object_ = self._api_request("session",**opt)
8184
self.session.ticket__ = self._ticket
82-
return object_
85+
return self
8386

8487
def delete(self,**opt):
8588
"""

0 commit comments

Comments
 (0)