Opened 7 months ago

Last modified 6 months ago

#33825 needs_revision defect

Make Environ handle "in" and "get()" like a dict

Reported by: teor Owned by:
Priority: Medium Milestone:
Component: Core Tor/Chutney Version:
Severity: Normal Keywords: ipv6, outreachy-ipv6, technical-debt
Cc: c Actual Points:
Parent ID: #33050 Points: 1
Reviewer: Sponsor: Sponsor55-can

Description

Some standard Python dict code doesn't work on chutney's Environ class:

is_in_env = 'foo' in self._env
value_or_none = self._env.get('foo')

"in" should return a boolean, and "get()" should return the value (or None).

But instead, when the key is missing, sometimes they throw a KeyError. (It seems to happen in certain contexts, but not others.)

We should work out if Environ is missing some of the required dict implementation functions. Or if there is some issue with Environ's lookup code.

Then we should implement unit tests, to make sure we don't break Environ in future.

Child Tickets

Change History (3)

comment:1 Changed 6 months ago by teor

Cc: c added; teor removed
Status: newneeds_revision

c made an initial pull request here, but it needs some fixes:
https://github.com/torproject/chutney/pull/67

comment:2 Changed 6 months ago by teor

For "in" (and "not in"), the object should define __contains__(), see:
https://docs.python.org/3.7/reference/expressions.html#membership-test-operations
https://docs.python.org/3.7/reference/datamodel.html#object.__contains

The easiest way to implement all the standard dict functions is to inherit from the MutableMapping abstract base class:
https://docs.python.org/3.7/library/collections.abc.html

If we define these functions, all other functions are implemented by the base class mixins:

__getitem__
__setitem__
__delitem__
__iter__
__len__

comment:3 Changed 6 months ago by c

I came across MutableMapping briefly, and after looking up how it compares to dict, yes, it does look like this would be more suitable because it defines a dict-like interface, which is likelier what we want here. It seems like this is the best way to achieve what we want.

Note: See TracTickets for help on using tickets.