> Looks good except an inline comment about an unused import.
Thanks. Ran flake8 and resolved 2 issues including the unused import.
> A little bit curious about if we could not use 'start' callbacks in the first
> ServiceManager instance for creating the slave. By then the service will have
> started, i guess.
Adding 'start' callbacks would be a way to make sure configure_slaves only runs after the jenkins service is started, but there would have to be a delay between the two as it takes jenkins a few seconds to be responsive. If this becomes a problem for the slave_manager, it could be addressed by adding a 'required_data' callback to ensure that jenkins is started and responding. After working with this for a while, I think it's easier to keep the relation and non-relation hooks handled separately, you don't need to re-install plugins when adding a slave for example.
> Looks good except an inline comment about an unused import.
Thanks. Ran flake8 and resolved 2 issues including the unused import.
> A little bit curious about if we could not use 'start' callbacks in the first
> ServiceManager instance for creating the slave. By then the service will have
> started, i guess.
Adding 'start' callbacks would be a way to make sure configure_slaves only runs after the jenkins service is started, but there would have to be a delay between the two as it takes jenkins a few seconds to be responsive. If this becomes a problem for the slave_manager, it could be addressed by adding a 'required_data' callback to ensure that jenkins is started and responding. After working with this for a while, I think it's easier to keep the relation and non-relation hooks handled separately, you don't need to re-install plugins when adding a slave for example.