-
Notifications
You must be signed in to change notification settings - Fork 2
feat(run-query.js): adds helix-run-query support #44
Conversation
can now embed query results from bigquery fix #9
src/matchers/run-query.js
Outdated
|
|
||
| async function extract(url, params, log = console) { | ||
| const host = 'https://adobeioruntime.net'; | ||
| const path = '/api/v1/web/helix/helix-services/run-query@latest/'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the major version number instead of latest, otherwise when you break the compatibility in run-query, you will bring data-embeds down with it.
src/matchers/run-query.js
Outdated
| statusCode: 200, | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'Cache-Control': 'max-age=600', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you get a Cache-Control header from the backend, you should propagate the value. Each query might have different caching characteristics. The 10 minute default cache is only used in places where we don't have any better information.
src/matchers/run-query.js
Outdated
| const errText = await results.text(); | ||
| log.error(`data request to ${resource} failed ${errText}`); | ||
| return { | ||
| statusCode: results.status, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use proper status code propagation. I'll provide a helper in Helix Shared that you can use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/matchers/run-query.js
Outdated
| statusCode: results.status, | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'Cache-Control': 'max-age=600', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, a default cache is useful.
…into support-big-query
use helix-shared for status code propagation fix #44
|
This PR will trigger a minor release when merged. |
Codecov Report
@@ Coverage Diff @@
## master #44 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 9 10 +1
Lines 259 279 +20
=========================================
+ Hits 259 279 +20
Continue to review full report at Codecov.
|
make data-embeds with _query/run-query pathname work, and unpin run-query api fix #44
trieloff
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Fix the build failure, and add a post-deploy integration test and then we are good to go.
…into support-big-query
wrap extract logic in try/catch also add post.deploy tests fix #44
wrap extract logic in try/catch also add post.deploy tests fix #44
e04088b to
53321a8
Compare
…into support-big-query
53321a8 to
5c98f7d
Compare
# [1.5.0](v1.4.3...v1.5.0) (2020-05-25) ### Bug Fixes * **run-query.js:** fix url pattern code ([5406c14](5406c14)), closes [#44](#44) * **run-query.js:** pass rev ([2b3a12f](2b3a12f)), closes [#44](#44) * **run-query.js:** wrap extract logic in try/catch ([cf3afd6](cf3afd6)), closes [#44](#44) ### Features * **run-query.js:** adds helix-run-query support ([8e30131](8e30131)), closes [#9](#9)
|
🎉 This PR is included in version 1.5.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
can now embed query results from bigquery
fix #9
Please ensure your pull request adheres to the following guidelines:
Related Issues
Thanks for contributing!