1237 - distributor config#262
Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
dkliban
left a comment
dkliban
left a comment
There was a problem hiding this comment.
Looks like you created your branch from master instead of 2.13-dev. There are 12 commits in this PR when only 1 is expected.
There was a problem hiding this comment.
@ammaritiz Thanks for this PR! 🎉
Please rebase this change to the 2.13-dev branch to avoid merging in changes from master.
| :rtype: tuple of length two. Either (False, str) or (True, None) | ||
| """ | ||
| path = config.get(constants.CONFIG_INSTALL_PATH) | ||
| if not path: |
There was a problem hiding this comment.
It looks like path is already being validated on line 100.
It definitely makes sense for the puppet install distributor to check that a install_path is provided so maybe just update the return value on line 102
| """ | ||
| path = config.get(constants.CONFIG_INSTALL_PATH) | ||
| if not path: | ||
| return False, _('config not specified') |
There was a problem hiding this comment.
The error message should be clearer, something like:
"An install_path has to be specified for the puppet install distributor."
ammaritiz
force-pushed
the
1237-distributor-config
branch
4 times, most recently
from
0c9b0a1 to
3a8ba60
Compare
|
There's also an unit test that needs to be updated: https://github.com/pulp/pulp_puppet/blob/master/pulp_puppet_plugins/test/unit/test_install_distributor.py#L93 |
ammaritiz
force-pushed
the
1237-distributor-config
branch
2 times, most recently
from
f304b49 to
af533cd
Compare
|
I have made the necessary changes. Please review. |
|
@ammaritiz This PR looks good. Consider adjusting the commit message to adhere to this form which has line wrapping. |
Config validation was returning True instead of False when a distributor_config was not specified while associating a distributor to a repository. closes #1237 https://pulp.plan.io/issues/1237
ammaritiz
force-pushed
the
1237-distributor-config
branch
from
af533cd to
34e1d5b
Compare
|
ok test |
5 participants
Added validation to check if distributor_config is specified while associating distributor to repository.
Issue 1237