Apply image changes 354 #922
Conversation
…one. This fixes the new application change detection code by making "inspected" application state have the same shape as "desired" application state - making both cases represent "no links" as an empty frozenset instead of making one of those cases use None to represent that.
This is necessary on top of just telling the FakeDockerclient that the unit has a volume. Without it the storage pool and the docker client have inconsistent volume state which is confusing and hopefully impossible in real life. It would be nice to have some nicer test scenario setup code so as to avoid the need for this duplicate state setup.
…ects that are returned by DockerClient in other words, horrible as it is, rely on RegEx parsing of environment variables in a container that match the format of <ALIAS>_PORT_<PORT>_TCP_PORT to derive the alias, local port (from the variable name) and remote port (from the variable value) for a Link. ** We do not have another way of doing this right now **
| external_port=portmap.external_port | ||
| )) | ||
| links = [] | ||
| link_pattern = re.compile( |
There was a problem hiding this comment.
I suggest moving this logic out into its own function or method and then unit testing it directly, in addition to the high-level test.
There was a problem hiding this comment.
Specifically the env variable parsing.
There was a problem hiding this comment.
Maybe instead of new function, just write relevant tests for application_from_units. Perhaps with some roundtripping to make sure you're using same environment variables we're generating elsewhere in the code.
|
Looking pretty good - please address comments and resubmit. |
| An ``Application`` with ``Link`` objects is discovered from a ``Unit`` | ||
| with environment variables that correspond to an exposed link. | ||
| """ | ||
| environment = Environment(variables=frozenset(( |
There was a problem hiding this comment.
Couldn't we create these env variables automatically, rather than by hardcoding, if the test ran a StartApplication with the expected Application?
Then merge. Thanks! |
…vironments and replace with links parsed in to environment via StartApplication
Fixes #353, #354 and #730 via the same code block.
See my comment on #730: #730 (comment)
in regards to deriving
Linkobjects onApplications; all we have fromDockerClientUnits are environment variables, such asPOSTGRES_PORT_5432_TCP_PORT=50432or something in that format. Therefore to apply changes to thelinksportion of an application configuration, the only way I can see to do it at the moment (assuming we're being selective about what gets restarted, which we currently are) is to do some pretty awful regex matching on those environment variables to extrapolate the originalLinks. I'm not happy with this approach, but someone's going to have to give me a better idea if they want this to change.Oh and #351 and/or #352 might be fixed by this branch, but if they are, it's a happy coincidence and work would certainly still need to be done on those issues at the very least in respect of tests.