Load sample when pipeline initially started #615
Conversation
| if !rows.Next() { | ||
| sql, args, err := sq. | ||
| Insert("system_infos"). | ||
| SetMap(sq.Eq{"IsSampleLoaded": false}). |
There was a problem hiding this comment.
Is this NoSQL in SQL? :)
Can you add some more description of what SystemInfoStore does? It sounds to me similar to what a config file does.
There was a problem hiding this comment.
it's actually only storing the status of the database. so i rename it to db_status_store. it would make more sense to be stored inside the db itself.
| if !rows.Next() { | ||
| sql, args, err := sq. | ||
| Insert("system_infos"). | ||
| SetMap(sq.Eq{"IsSampleLoaded": false}). |
There was a problem hiding this comment.
nit: Can we move the map to a constant at the top of the file, since this is basically the system's default config?
| rows, err := tx.Query(getSystemInfoSql, getSystemInfoArgs...) | ||
| if err != nil { | ||
| tx.Rollback() | ||
| return util.NewInternalServerError(err, "Failed to get load sample status") |
There was a problem hiding this comment.
Maybe this should say "Failed to load system information/state/..."
| } | ||
|
|
||
| func (s *SystemInfoStore) InitializeSystemInfoTable() error { | ||
| getSystemInfoSql, getSystemInfoArgs, err := sq.Select("*").From("system_infos").ToSql() |
There was a problem hiding this comment.
s/system_infos/system_configmap/? or maybe system_config?
| ) | ||
|
|
||
| type SystemInfoStoreInterface interface { | ||
| IsSampleLoaded() (bool, error) |
There was a problem hiding this comment.
nit: maybe haveSamplesLoaded?
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: IronPan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Fix #615 |
* add pytest * install kubernetes * install google-api-client & yq * install google-cloud * update * update img to pass kfctl e2e test * add httplib * remove httplib * sort
* Return application/json as content-type header * Call validate after preprocess in ExplainHandler * Add new line at end of file
Instead of using a K8s Job to load the sample, this change load the sample when pipeline backend is started and create a permanent system info table to store the state, so we don't load them again in case of pod failover or during upgrade.
The K8s Job doesn't work well because it's immutable once created, ksonnet throw an error of "merging object with existing state failure" in case of upgrading the system.
The system info table will potentially also be useful in the future for storing the debugging information such as versions, upgrade history, DB version etc.
This change is