irods / python-irodsclient

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

[#547] Fix error message for non str metadata in add #548

Closed mstfdkmn closed 2 months ago

mstfdkmn commented 2 months ago

Looks like checking atr and val type before checking length is enough to see the same error like the set method raises.

alanking commented 2 months ago

@mstfdkmn - Any reason you closed this?

alanking commented 2 months ago

Oh, maybe superceded by https://github.com/irods/python-irodsclient/pull/552? @d-w-moore - Please advise.

d-w-moore commented 2 months ago

Oh, maybe superceded by #552? @d-w-moore - Please advise.

@mstfdkmn Yes, I think #552 may solve #547 to your satisfaction and plausibly supercede the present PR. Note though, an alternative class of exception (irods.message.Bad_AVU_Field) is thrown in place of the old ValueError that was thrown by add ( ) for empty attributes or values.

See this comment. I've deleted the code in metadata addmethod because it's redundant to the newer code embedded in the MetadataRequest constructor (a more recent and perhaps better solution than having the checks located in the various high-level methods aka set and add ...)

On another point:

I don't know if it makes sense to make Bad_AVU_Field descend from ValueError - it's conceivable that some applications might be happier that way, if they now depended on the ValueError thrown previously by add( ). But really, as I was pointing out in the referrred-to comment,So far, though, I've left it as a direct subclass of Exception. @mstfdkmn feel free to express a preference if you have one..

mstfdkmn commented 2 months ago

Yes, I see the same solution like mentioned above and have no preference. https://github.com/irods/python-irodsclient/pull/548#discussion_r1585435663