-
Notifications
You must be signed in to change notification settings - Fork 122
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
feat: Add Google BigQuery data loader #1543
feat: Add Google BigQuery data loader #1543
Conversation
Let me know if anything else is missing, then I can include it :) |
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.
Thanks, this is a very useful addition!
I think you can simplify it quite a bit by having only one page (index.md
), and by taking full advantage of Framework's data loader and Plot's defaults to trim a lot of the code. (I can do it if you prefer.)
@@ -0,0 +1,7 @@ | |||
[Framework examples →](../) |
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.
this file needs to be updated
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.
Something like below could work? Just need to test that the "view live" when final changes is merged :)
Google BigQuery Data Loader
View live: https://observablehq.observablehq.cloud/framework-example-loader-google-bigquery/
This Observable Framework example demonstrates how to write a JavaScript data loader that fetches metrics from the Google Bigquery API using the Google BigQuery Data Node.js Client. The data loader fetches the confirmed_cases
metric (based on a publicly available Covid-19 BigQuery dataset) for a specified date range and then displays the data in a line chart. The data loader lives in src/data/covidstats_it.csv.js
and uses the helper src/data/google-bigquery.js
.
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!
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.
Will you directly do all suggested trims/changes and merge after (if possible)? I don't mind, let me know what works best for you / what will be the fastest for you 🙏
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.
yes I can take it from here
|
||
```js echo | ||
display( | ||
Plot.plot({ |
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.
That's a bit too much code for a simple line chart, we can trim it down quite a bit (I can help with that).
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.
Yes, can you trim please 🙏
``` | ||
|
||
### The output should look as follows: | ||
![image info](covid_stats.png) |
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.
I don't think it's necessary to add a static image since the chart is displayed just above?
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.
Agreed :) The reason why I added it, is that it is always night to upfront see what users want to replicate and whether the result what they need to achieve is the same. Not absolutely necessary and might even be better to move this image to the README.
console.log("Parsed Data:", parsedData); | ||
|
||
} catch (error) { | ||
console.error('Error running query or processing file:', error); |
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.
It would be simpler (and, more importantly, better) to not try/catch, but let the loader error naturally; this way Framework would know that it has errored, would not cache the result, and would stop the build (if building), or retry (if in preview).
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.
Can be removed yes 🙏
I actually mirrored the Google Analytics Loader file/dir-setup, but if you think it is helpful to keep it simpler, let's go with that route rather 🙏 |
Co-authored-by: Philippe Rivière <[email protected]>
Co-authored-by: Philippe Rivière <[email protected]>
Extended the loaders by adding a Google BigQuery loader with a concrete example on how data can be used. I hope this can also helpful for others to :)