Skip to content

Conversation

beniwohli
Copy link
Contributor

@beniwohli beniwohli commented Aug 5, 2019

fixes #459

Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few little things, otherwise looks good.

def __set__(self, instance, value):
if isinstance(value, compat.string_types):
items = (item.split(self.keyval_separator) for item in value.split(self.item_separator))
value = {key: self.type(val) for key, val in items}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should strip key and val. Someone's bound to have white space in their environment variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 06b64dc

elif not isinstance(value, dict):
# TODO: better error handling
value = None
instance._values[self.dict_key] = value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably check that the tag keys match TAG_RE here? also, truncate value to 1024 chars?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both done in 06b64dc

@beniwohli beniwohli closed this in 6113dc2 Aug 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for global labels
2 participants