bpo-44242: [Enum] remove missing bits test from Flag creation

[ethanfurman]

Move the check for missing named flags in composite flag aliases from
Flag creation to a new verify decorator.

https://bugs.python.org/issue44242

[tacaswell]

Is there any guidance on when to use @unique vs @verify(UNIQUE)?

[ethanfurman]

The only difference between the two is unique raises ValueError and verify raises TypeError. I don't see unique going away, so it's pretty much personal preference.

[ethanfurman]

Changed that so both raise ValueError. I added UNIQUE to verify() for two reasons:

• so multiple checks would not require multiple decorators
• so unique() could eventually go away (not that it will any time soon)

[miss-islington]

Thanks @ethanfurman for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

[miss-islington]

Sorry, @ethanfurman, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker eea8148b7dff5ffc7b84433859ac819b1d92a74d 3.10

[akulakov]

Would it make sense to limit missing to missing[:100] or so? It can be arbitrarily large and it's not obvious to users that a large flag value can create a very large exception (and log msg) here. [edit: should have reloaded, didn't realize it's already merged]

[ethanfurman]

Yes it would.

[akulakov]

I can make a PR on the same issue, if that's okay?

[ethanfurman]

[I merged it after your comment, as it fixes a regression; we can still make your change if needed.]

Having thought about it a little more, I'm not sure we need that: For any static enum, and probably most dynamic ones, the exception will happen the first time the .py file is loaded and not at some random time on the end-users machine. Which specific scenarios were you thinking about, and is your experience different?

[akulakov]

1. For some subset of dynamic Enums where let's say a developer tests it with small values and the app logic logs an error and proceeds. In actual use the dynamic value might be calculated or loaded from somewhere and might be much larger.
2. If a value is calculated or loaded from a web request, it can be used as a DoS attack or to overfill the log files.

Both seem pretty unlikely so I'm not sure it's a needed fix, just wanted to point it out.

[ethanfurman]

Good points. I changed the error message to list the problem aliases and the combined value of missing flags, then limited that to 256 characters.

Thank you for your help!

[bedevere-bot]

GH-26635 is a backport of this pull request to the 3.10 branch.

[akulakov]

Perfect, thank you!

…

On Thu, Jun 10, 2021 at 3:29 PM Ethan Furman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In Lib/enum.py <#26586 (comment)>: > + missing = [] + if enum_type == 'flag': + # check for powers of two + for i in range(_high_bit(low)+1, _high_bit(high)): + if 2**i not in values: + missing.append(2**i) + elif enum_type == 'enum': + # check for powers of one + for i in range(low+1, high): + if i not in values: + missing.append(i) + else: + raise Exception('verify: unknown type %r' % enum_type) + if missing: + raise ValueError('invalid %s %r: missing values %s' % ( + enum_type, cls_name, ', '.join((str(m) for m in missing))) Good points. I changed the error message to list the problem aliases and the combined value of missing flags, then limited that to 256 characters. Thank you for your help! — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#26586 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB3RIQAYK5AYXHT7E3ZIKTTSEHA3ANCNFSM46IT353Q> .