Wrong parameter passed to `ConfigVars.new_from_dict()` in `ConfigVars.update()`

[cans]

In the method ConfigVars.update() a new ConfigVars instance is built, with the values returned by the heroku API, and returned to the user. Yet that instance is not valid as the h keyword argument receives a ConfigVars instance instead of the expected Heroku instance (passed self instead of self._h).

Given that:

• this issue has not been reported before (meaning most likely no one ever used the return value of that method in its code);
• this class tries very hard to look like a dict;
• the dict.update() method has a semantic that mutates the dictionary state in place and returns None (as all function / method mutating built-in types do in Python);

I would suggest that the API be modified as follows: the state of the ConfigVars instance should mutate it's internal dictionary (self.data) upon successful response from Heroku's API, and the method returns None.

[cans]

There are lots of similar oddities in that module's code:

• __delitem__ should raise KeyError or some other exception. Not return a boolean;
• Most method use self._h._resource_serialize(...) to serialize their input before sending it to Heroku. But the __setitem__() method doesn't and uses the json.dumps() function directly;
• to_dict() exposes the internal state of the object: it return self.data meaning a code like this:

config_vars = app.config()
d = config_vars.to_dict()  # or === config_vars.dict()
# From here on d and self.data reference the *same* mutable data structure !!
del d['SOME_SETTING']
if 'SOME_SETTING' not in config_vars:
    print('Oh My !')

Would print 'Oh My !'. I am not sure this is a good idea. One should copy the dictionary referenced by self.data before passing it on to the user. Otherwise, the state of the object is inconsistent with the actual state of the application as stored in Heroku infrastructure.

[martyzz1]

Thanks Nicolas, this is really helpful, and appreciated.