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

Poll for results in parameterized embeds #3752

Merged
merged 23 commits into from
May 6, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ae33080
add an endpoint for fetching job using a query's api_key
May 1, 2019
92d3c31
when unauthenticated, use api_key to get job, and fetch the latest query
May 1, 2019
c16263c
add 'refresh dataset' button to parameters directive
May 1, 2019
2d4b5fa
Merge branch 'master' into poll-for-results-in-embeds
May 1, 2019
41b0d81
fix scope error introduced by earlier commit
May 1, 2019
5653bd4
show timer when refreshing results
May 2, 2019
bbc54e3
Show input for missing parameters in embedded visualizations (#3741)
May 2, 2019
1968b49
Merge branch 'master' into poll-for-results-in-embeds
May 2, 2019
740472b
don't render the execute button for each parameter
May 2, 2019
d8bf19d
Merge branch 'poll-for-results-in-embeds' of github.com:getredash/red…
May 2, 2019
7389725
show 'missing parameter value' error
May 2, 2019
7d7414f
Don't reload the whole page when parameter value changes.
arikfr May 5, 2019
6651edf
Set API key and load config before rendering.
arikfr May 5, 2019
9915d88
Add Query#hasParameters method.
arikfr May 5, 2019
43e3abf
Don't show download controls for parameterized queries (they won't wo…
arikfr May 5, 2019
70da22d
Use getUrl to construct a correct query link.
arikfr May 5, 2019
7aa89dc
WIP: have a single way to load results
arikfr May 5, 2019
9841373
Show persistent errors and finish loading logic.
arikfr May 5, 2019
b6620cb
Check if query is safe and show message otherwise.
arikfr May 5, 2019
7ffd7eb
Fix test for unsafe parameters embed.
arikfr May 5, 2019
dadbe73
Merge branch 'master' into poll-for-results-in-embeds
May 5, 2019
13c4d45
Merge branch 'poll-for-results-in-embeds' of github.com:getredash/red…
May 5, 2019
55be3c6
wait for query results to return before taking snapshot
May 5, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions client/app/components/parameters.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,8 @@
<i class="zmdi zmdi-settings"></i>
</button>
<parameter-value-input param="param"></parameter-value-input>
<button class="m-l-5 btn btn-primary" ng-if="onRefresh" ng-click="onRefresh()" title="Refresh Dataset">
<span class="zmdi zmdi-play"></span>
</button>
Copy link
Member

Choose a reason for hiding this comment

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

Considering this just executes query refresh, why not put it outside the parameters component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parameter directive's containing div is a block, so putting it outside would cause the button to drop below.

Copy link
Member

Choose a reason for hiding this comment

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

🤔 not ideal, but it's temporary, so YOLO... maybe add a comment for @gabrieldutra to remove this once he's done with #3737 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙄
hey, @gabrieldutra! remove this once you're done with #3737!

Copy link
Member

Choose a reason for hiding this comment

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

</div>
</div>
1 change: 1 addition & 0 deletions client/app/components/parameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ function ParametersDirective($location) {
editable: '=?',
changed: '&onChange',
onUpdated: '=',
onRefresh: '=',
},
template,
link(scope) {
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/queries/visualization-embed.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ <h3>

<div class="col-md-12 query__vis">
<div class="p-t-15 p-b-5" ng-if="$ctrl.query.getParametersDefs().length > 0">
<parameters parameters="$ctrl.query.getParametersDefs()"></parameters>
<parameters parameters="$ctrl.query.getParametersDefs()" on-refresh="$ctrl.refreshQueryResults"></parameters>
</div>

<visualization-renderer visualization="$ctrl.visualization" query-result="$ctrl.queryResult" class="t-body">
Expand Down
4 changes: 4 additions & 0 deletions client/app/components/queries/visualization-embed.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ const VisualizationEmbed = {
this.queryResult = new QueryResult(this.data[1]);
this.visualization =
find(this.query.visualizations, visualization => visualization.id === visualizationId);

this.refreshQueryResults = () => {
this.queryResult = this.query.getQueryResult();
};
},
};

Expand Down
28 changes: 20 additions & 8 deletions client/app/services/query-result.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ function getColumnFriendlyName(column) {
return getColumnNameWithoutType(column).replace(/(?:^|\s)\S/g, a => a.toUpperCase());
}

function QueryResultService($resource, $timeout, $q, QueryResultError) {
function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) {
const QueryResultResource = $resource('api/query_results/:id', { id: '@id' }, { post: { method: 'POST' } });
const Job = $resource('api/jobs/:id', { id: '@id' });
const JobWithApiKey = $resource('api/queries/:queryId/jobs/:id', { queryId: '@queryId', id: '@id' });
const statuses = {
1: 'waiting',
2: 'processing',
Expand Down Expand Up @@ -293,6 +294,14 @@ function QueryResultService($resource, $timeout, $q, QueryResultError) {
return queryResult;
}

loadLatestCachedResult(queryId, parameters) {
$resource('api/queries/:id/results', { id: '@queryId' }, { post: { method: 'POST' } })
.post({ queryId, parameters },
this.update,
(response) => { this.update(response); },
(error) => { handleErrorResponse(this, error); });
}

loadResult(tryCount) {
this.isLoadingResult = true;
QueryResultResource.get(
Expand Down Expand Up @@ -324,18 +333,21 @@ function QueryResultService($resource, $timeout, $q, QueryResultError) {
);
}

refreshStatus(query, tryNumber = 1) {
Job.get(
{ id: this.job.id },
refreshStatus(query, parameters, tryNumber = 1) {
const resource = Auth.isAuthenticated() ? Job : JobWithApiKey;
const loadResult = Auth.isAuthenticated() ?
this.loadResult : () => this.loadLatestCachedResult(query, parameters);
resource.get(
{ queryId: query, id: this.job.id },
(jobResponse) => {
this.update(jobResponse);

if (this.getStatus() === 'processing' && this.job.query_result_id && this.job.query_result_id !== 'None') {
this.loadResult();
loadResult();
} else if (this.getStatus() !== 'failed') {
const waitTime = tryNumber > 10 ? 3000 : 500;
$timeout(() => {
this.refreshStatus(query, tryNumber + 1);
this.refreshStatus(query, parameters, tryNumber + 1);
}, waitTime);
}
},
Expand Down Expand Up @@ -377,7 +389,7 @@ function QueryResultService($resource, $timeout, $q, QueryResultError) {
queryResult.update(response);

if ('job' in response) {
queryResult.refreshStatus(id);
queryResult.refreshStatus(id, parameters);
}
},
(error) => {
Expand Down Expand Up @@ -408,7 +420,7 @@ function QueryResultService($resource, $timeout, $q, QueryResultError) {
queryResult.update(response);

if ('job' in response) {
queryResult.refreshStatus(query);
queryResult.refreshStatus(query, parameters);
}
},
(error) => {
Expand Down
5 changes: 4 additions & 1 deletion redash/handlers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ def json_representation(data, code, headers=None):
'/api/queries/<query_id>/results.<filetype>',
'/api/queries/<query_id>/results/<query_result_id>.<filetype>',
endpoint='query_result')
api.add_org_resource(JobResource, '/api/jobs/<job_id>', endpoint='job')
api.add_org_resource(JobResource,
'/api/jobs/<job_id>',
'/api/queries/<query_id>/jobs/<job_id>',
endpoint='job')

api.add_org_resource(UserListResource, '/api/users', endpoint='users')
api.add_org_resource(UserResource, '/api/users/<user_id>', endpoint='user')
Expand Down
2 changes: 1 addition & 1 deletion redash/handlers/query_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ def make_excel_response(query_result):


class JobResource(BaseResource):
def get(self, job_id):
def get(self, job_id, query_id=None):
"""
Retrieve info about a running query job.
"""
Expand Down