search

dcantrell / pyparted

Python bindings for GNU parted (libparted)
GNU General Public License v2.0
86 stars 43 forks source link

exception on attempted access to partition name on non-gpt is cumbersome #59

Closed dwlehman closed 2 years ago

dwlehman commented 5 years ago

I know I'm late to this party, but my opinion is that raising an exception on 'get' of this property makes it unnecessarily difficult for client code. Setting it? Sure. For the getter, just return None or ''.

(FWIW I found out about this change here.)

Of course I can make the change in blivet, but I thought it was worth whining about it first.

dcantrell commented 5 years ago

Apologies for the late reply here. I don't know why, but for just pyparted I was not getting notifications about issues. I think I have fixed that.

@dwlehman what was it that you're asking for here? I've lost context.

dwlehman commented 5 years ago

The change in dcantrell/pyparted#34 led to a fairly widely-reported anaconda/blivet traceback (https://bugzilla.redhat.com/show_bug.cgi?id=1723228). When the blivet code was written pyparted would return None on unsupported disklabels, but with that change it started to raise an exception instead. I opened this because I prefer the old behavior, at least for the getter; for the setter it makes more sense to crash.

dcantrell commented 5 years ago

That makes sense to me. Return None on the getter but retain the exception raising for the setter. Sorry about that. Looking back through everything, I remember someone reporting the incomplete exposure of the partition name support, which I felt was valid.

Next time, please do file an issue here or send a PR. I never was aware of BZ #1723228.

dcantrell commented 5 years ago

I'm going to push a candidate fix for this to master. If you can try it out and post here if it's what you're looking for, that'd help.

dcantrell commented 5 years ago

@dwlehman Able to try the proposed patch on master?

bcl commented 5 years ago

Tested the new pyparted on a rawhide system, by hand observing that it raised the error before installing the new version from master and observing it returns None. Also tested with livemedia-creator --make-disk to make sure nothing unexpected happens. Works fine for me.

dwlehman commented 5 years ago

Thanks @bcl -- I have been out for most of the past week and was hoping to test it this week.

dcantrell commented 2 years ago

Based on the comment from @bcl, I'm closing this issue. Fix will be in 3.12.0.