python-irodsclient icon indicating copy to clipboard operation
python-irodsclient copied to clipboard

Consider moving `irods.test.helpers.make_session` into `irods.helpers`

Open alanking opened this issue 1 year ago • 6 comments

It seems that this irods.test.helpers.make_session function has grown to be useful beyond just the tests. Should we move it into this module so that it can be used directly? If so, let's create an issue to look into that some other time (i.e. don't do it as part of this change). The existing version in irods.test.helpers can then call the new one with its existing default value(s) to minimize the blast radius.

Originally posted by @alanking in https://github.com/irods/python-irodsclient/pull/589#discussion_r1686700667

It may or may not be beneficial to do this, but I didn't want to lose the idea, so here's the issue. :)

alanking avatar Jul 23 '24 15:07 alanking

FYI: I remember this https://github.com/irods/python-irodsclient/issues/412 because waiting for an enhancement to use make_session easily where needed.

mstfdkmn avatar Jul 24 '24 11:07 mstfdkmn

@mstfdkmn - Good catch. Perhaps this can be closed as a duplicate of #412. Thoughts, @d-w-moore?

alanking avatar Jul 24 '24 14:07 alanking

Yes, this PR probably sets us on a particular path for addressing #412. As to what's the better name, irods.utility, irods.misc or irods.helpers, I don't know. I usually defer to the consensus. Have there been any other PR's around that attempt to establish a common space for utility functions and such?

d-w-moore avatar Jul 24 '24 16:07 d-w-moore

There's already an irods.session module. Why not put it in there?

from irods.session import make_session

korydraughn avatar Jul 24 '24 16:07 korydraughn

@korydraughn I'm ok with that. So then we keep #412 around for now, and consider whether it may or may not be addressed by the creation of irods.helpers.xml_mode and the namespace / package in which it resides?

d-w-moore avatar Jul 24 '24 16:07 d-w-moore

Sounds good to me.

korydraughn avatar Jul 24 '24 17:07 korydraughn