Azure / azure-sdk-tools

Tools repository leveraged by the Azure SDK team.
MIT License
113 stars 177 forks source link

Duplicate values in an enum cause the key to be invisible. API Review fails with "breaking changes" as a result. #1601

Open scbedd opened 3 years ago

scbedd commented 3 years ago

This PR for eventgrid added backwards compatible keys to the SystemEventNames enum. Reference these duplicate values, not keys.

However, python an efficiency hack built in to the Enum class. Specifically, when a key with a duplicate value is seen, it is simply aliased to the first key with that value. It is not actually visible in a inspect() or other debugging methodology.

For instance, the SystemEventNames enum in eventgrid has the following values (extracted for brevity)

...
AcsChatParticipantAddedToThreadEventName = 'Microsoft.Communication.ChatThreadParticipantAdded'
...
# backwards compat
AcsChatThreadParticipantAddedEventName = 'Microsoft.Communication.ChatThreadParticipantAdded'
...

The newer key name is first in the enum. However, if you were to inspect this object at runtime, you would not see the second key name as a member of the object. However, if you were access it specifically via

from azure.eventgrid import SystemEventNames
print(SystemEventNames.AcsChatThreadParticipantAddedEventName)

You'd actually get a value! This is because these duplicated values are merely aliased. NOT actually part of the object tree. The API Review tool should probably handle this edge case.

Discussion on Stack Overflow

scbedd commented 3 years ago

It does show up in inspect.getmembers when called on the enum.

from azure.eventgrid import SystemEventNames

result = inspect.getmembers(SystemEventNames)
members = [member_result for member_result in result if member_result[0] == "__members__"]

The resulting object you get back from members is a tuple with ('__members__', mappingproxy(<enum pairs>)

The key here is that the values will be set the same, but the key is definitely still available.

Here's the exact line of enum.py that does this.

# If another member with the same value was already defined, the
# new member becomes an alias to the existing one.
for name, canonical_member in enum_class._member_map_.items():
    if canonical_member._value_ == enum_member._value_:
        enum_member = canonical_member
        break
else:
    # Aliases don't appear in member names (only in __members__).
    enum_class._member_names_.append(member_name)