Dump running Drucker service configurations for backup #7
Conversation
keigohtr
requested review from
sugyan,
yoquankara,
yustoris,
jkw552403,
yuki-mt and
shimpei-yotsukura
|
I have 2 comments:
|
|
Thank you for the comments.
There are 4 types of configuration this PR dumps.
Sure. As you know, @jkw552403 san are creating unit test. I will add the test after merging his commit. (So this PR will be merged after merging his commit) |
|
Thanks for your explanation! So I got that with those 4 fundamental configurations we can recreate service in case of emergency. Looking forward to unit test. |
…s-option Merged
Refactor architecture
…cker/drucker-dashboard into feature/dump-kubernetes-yaml
….com/drucker/drucker-dashboard into feature/dump-kubernetes-yaml" This reverts commit 0e60cac, reversing changes made to 0acfe39.
|
@yoquankara |
| :return: | ||
| """ | ||
| mobj = db.session.query(Model).filter( | ||
| Model.application_id==application_id,Model.model_id==model_id).one() |
There was a problem hiding this comment.
I think exception NoResultFound should be handled or logged in case none is found.
There was a problem hiding this comment.
Exceptions are caught by api.errorhandler.
So we can handle and log them.
There was a problem hiding this comment.
I see. I still prefer explicit error handling at the right place because it is under our control and enable us to fix quickly with more specific log, instead of stack trace in default error handler.
If we decide to follow above practice, review of whole code base is necessary though. Maybe it should be a TODO.
There was a problem hiding this comment.
Cloud you create Issue about this?
Error handling is one of the most important things. I agree with your opinion!
At this time, this PR is not for refactoring. Thank you.
| Model.application_id==application_id,Model.model_id==model_id).one() | ||
| model_path = mobj.model_path | ||
| sobj = db.session.query(Service).filter( | ||
| Service.application_id == application_id, Service.service_id==service_id).one() |
| sobj.model_id = model_id | ||
| db.session.flush() | ||
|
|
||
| aobj = db.session.query(Application).filter(Application.application_id == application_id).one() |
| @@ -747,6 +763,145 @@ def create_or_update_drucker_on_kubernetes( | |||
| body=v2_beta1_horizontal_pod_autoscaler | |||
| ) | |||
| """ | |||
| return service_name | |||
|
|
|||
| def switch_drucker_service_model_assignment( | |||
There was a problem hiding this comment.
It would be great if unit test for this method could also be added
There was a problem hiding this comment.
Yes I think so.
But this PR is not for modifying the unit test. We will create another one for this.
What is this PR for?
For backup.
If Kubernetes crushes completely, we will lost all existing Drucker service configurations.
For a service recovery, this PR dumps Drucker service configurations.
This PR includes
kube.datadirin settings.ymlDIR_KUBE_CONFIGdirectoryWhat type of PR is it?
Feature
What is the issue?
N/A
How should this be tested?
Do "Sync" on Drucker-dashboard