Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the List run to get all runs #583

Merged
merged 1 commit into from
Dec 21, 2018
Merged

Conversation

IronPan
Copy link
Member

@IronPan IronPan commented Dec 21, 2018

We now don't assume runs always have resource reference, the sql query need to update to avoid assuming it exist. Otherwise the runs created not inside an experiment won't show up in the list run view.

Query change
Before

SELECT subq.*, 
       "[" 
       || Group_concat(r.payload, ",") 
       || "]" AS refs 
FROM   (SELECT rd.*, 
               "[" 
               || Group_concat(m.payload, ",") 
               || "]" AS metrics 
        FROM   run_details AS rd 
               LEFT JOIN run_metrics AS m 
                      ON rd.uuid = m.runuuid 
        GROUP  BY rd.uuid) AS subq 
       LEFT JOIN resource_references AS r 
              ON subq.uuid = r.resourceuuid
	   WHERE  r.resourcetype = 'Run'
GROUP  BY subq.uuid 
ORDER  BY createdatinsec ASC, 
          uuid ASC 
LIMIT  21 

After

SELECT subq.*, 
       "[" 
       || Group_concat(r.payload, ",") 
       || "]" AS refs 
FROM   (SELECT rd.*, 
               "[" 
               || Group_concat(m.payload, ",") 
               || "]" AS metrics 
        FROM   run_details AS rd 
               LEFT JOIN run_metrics AS m 
                      ON rd.uuid = m.runuuid 
        GROUP  BY rd.uuid) AS subq 
       LEFT JOIN (SELECT * 
                  FROM   resource_references 
                  WHERE  resourcetype = 'Run') AS r 
              ON subq.uuid = r.resourceuuid 
GROUP  BY subq.uuid 
ORDER  BY createdatinsec ASC, 
          uuid ASC 
LIMIT  21 

This change is Reviewable

@IronPan
Copy link
Member Author

IronPan commented Dec 21, 2018

/assign @yebrahim

Copy link
Contributor

@yebrahim yebrahim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
Thanks for fixing this.

@IronPan
Copy link
Member Author

IronPan commented Dec 21, 2018

/approve

@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 66d238d into kubeflow:master Dec 21, 2018
Linchin pushed a commit to Linchin/pipelines that referenced this pull request Apr 11, 2023
…ubeflow#583)

* Instead of using ksonnet to manage the PV and PVC for NFS switch
  to using kustomize and update the playbook.

Fix kubeflow#582
magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this pull request Oct 22, 2023
…a event source (kubeflow#583)

* Fix flaky test for deleting canary

* Add addressable duck type

* Support internal route

* Fix internal route host

* Move validate after preprocess

* Separate load json

* Fix response

* clear output

* Fix tests

* Fix predictor host

* Fix predictor host

* Fix virtual service tests

* Fix controller tests

* Add more instructions

* Create external service for the virtual service host

* Use localgateway host constant

* Update kafka example doc

* Add isvc addressable cluster role

* Add diagram

* Reference helm3

* Fix v1 type merge

* Address comment

* Remove notebook

* Add kustomize requirement
HumairAK pushed a commit to red-hat-data-services/data-science-pipelines that referenced this pull request Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants