Code review comment for lp:~frankban/juju-quickstart/new-auth-api-endpoint

Revision history for this message
Francesco Banconi (frankban) wrote :

Thanks for the reviews!

https://codereview.appspot.com/199490043/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/199490043/diff/1/quickstart/app.py#newcode283
quickstart/app.py:283: def get_env_uuid_or_none(env_name):
On 2015/02/10 10:47:32, martin.hilton wrote:
> Is it really necessary to have or_none in the name of this function?

Not strictly necessary, but in the caller context this can help: while
most of the times app functions return values, this can also return
none, i.e. do not rely on the fact the env uuid is always known.

https://codereview.appspot.com/199490043/diff/1/quickstart/models/jenv.py
File quickstart/models/jenv.py (right):

https://codereview.appspot.com/199490043/diff/1/quickstart/models/jenv.py#newcode134
quickstart/models/jenv.py:134: data =
serializers.yaml_load_from_path(jenv_path)
On 2015/02/10 15:12:09, jeff.pihach wrote:
> I believe there are other places in the code which require information
from the
> jenv file so I figured that this would already be abstracted out into
a utility
> method already so you could just fetch the value.

A slight refactor of the code in the jenv models can help, I agree.
On the other hand, the repeated code is still just two lines, so perhaps
not something for this branch.

https://codereview.appspot.com/199490043/

« Back to merge proposal