Return resource count from ListXXX calls #595
Conversation
|
fix #103 |
|
/assign @IronPan |
yebrahim
force-pushed
the
resource-count
branch
from
b3e5545 to
0858e22
Compare
|
comments should be resolved now, PTAL. |
yebrahim
force-pushed
the
resource-count
branch
2 times, most recently
from
dbb7438 to
f5b3d7d
Compare
|
/test kubeflow-pipeline-build-image |
yebrahim
force-pushed
the
resource-count
branch
from
d8880f4 to
48ecfad
Compare
| } | ||
| if len(results) < newContext.PageSize { | ||
| return results, "", nil | ||
| func FilterOnResourceReference(tableName string, resourceType common.ResourceType, selectCount bool, |
There was a problem hiding this comment.
how about passing "*" and "count(*)" string instead of selectCount boolean flag. maybe consider define them as const? it would be more readable from method caller perspective.
There was a problem hiding this comment.
I can do that, but selectCount is passed through from the different implementations of buildSelectXXXQuery. I think that's a better interface there, since it's being relied on to change the way the query is built. If you really want this change, do you think it should be propagated to all 4 implementations?
| return sqlBuilder.From("pipelines").Where(sq.Eq{"Status": model.PipelineReady}) | ||
| } | ||
|
|
||
| sqlBuilder := buildQuery(sq.Select("*")) |
There was a problem hiding this comment.
please consider define "*" and "count(*)" as const and use them here
There was a problem hiding this comment.
I don't think this adds any safety, but it risks making the code less readable IMO.
What would you call these variables? Also, do you think we should make the rename everywhere?
|
please add integration tests for covering corresponding new API features. |
yebrahim
force-pushed
the
resource-count
branch
3 times, most recently
from
24c9408 to
d4d996c
Compare
yebrahim
force-pushed
the
resource-count
branch
from
d4d996c to
a96fc14
Compare
yebrahim
force-pushed
the
resource-count
branch
from
1ca6e1d to
55bacd1
Compare
|
@IronPan changed integration tests to check TotalSize as well, PTAL. |
|
/lgtm |
|
/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 |
1 similar comment
|
[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 |
* See kubeflow#595 * This PR is creating a reconciler for the auto deployed clusters. * The reconciler compares the list of auto-deployed clusters against the master and release branches to determine if we need a cluster based off a newer commit. * If we do we fire off a K8s job to deploy Kubeflow. * This PR includes some general utilities * assertions.py some useful utilities for writing test assertions to compare lists and dictionaries * gcp_util some common GCP functionality like an iterator to list Deployments. * git_repo_manager.py a wrapper class to manage a local clone of a git repo.
* Create a new Kubeflow cluster to run auto-deploy v2. * kubeflow#595 we are creating a new version of the infrastructure to auto-deploy Kubeflow from master and release branches * This PR contains the GCP and Kustomize manifests for setting up a new Kubeflow cluster where this app will run. * We can't use the existing kubeflow-testing cluster because that doesn't have ISTIO or workload identity which we want for the auto-deploy app. * Remove OWNERs files
* Create skaffold configs and kustomize configs for deploying the auto_deployer. * Create a Dockerfile which just copies the source to try to allow fast rebuilds * Delete files for the old auto-deploy jobs. * Modify create_unique_kf_instance to allow setting labels that can be used to identify the auto-deployment. Miscellaneuous * Update the playbook about quota errors; unrelated to this PR but useful. Fix kubeflow#595 - AutoDeply V2
* Add a README explaining how auto deploy works. * Delete the old cron jobs since these are no longer used. Related to kubeflow#595
Resolves kubeflow#595 Signed-off-by: Christian Kadner <[email protected]>
Fixes #103.
Adds a second SQL query in the server list objects paths, which runs in a transaction to count matching objects, accounting for filter predicates, but not pagination, and adds a
total_sizefield in list response objects to return the number. Unit and integration tests were updated to make sure the right numbers are returned./area back-end
This change is