irods / python-irodsclient

A Python API for iRODS
Other
62 stars 73 forks source link

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

Open alanking opened 4 months ago

alanking commented 4 months ago

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. :)

mstfdkmn commented 4 months ago

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

alanking commented 4 months ago

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

d-w-moore commented 4 months ago

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?

korydraughn commented 4 months ago

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

from irods.session import make_session
d-w-moore commented 4 months ago

@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?

korydraughn commented 4 months ago

Sounds good to me.