Fix blog build #891
Fix blog build #891
Conversation
minhkhul
requested review from
rlunde,
melange396 and
carlynvandyke
as code owners
✅ Deploy Preview for cmu-delphi-main ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
-
Nice job getting this to build!
-
I made a number of in-line comments regarding whitespace/formatting consistency (but they are mostly inconsequential), and one comment for verbiage that i think is important.
-
I don't know how we prevented it from being displayed before, but now the "template post" page is showing up in the netlify preview on the list of blog posts and as the article itself. It does not show on the current live site blog post list and the article page does not exist there either. We could delete the template file entirely to solve this, but i think it is probably worthwhile to keep as a reference if possible.
-
Are we sure we want to regenerate [all of] these blog post pages?
-
For instance, see the significant diff for the image file
static/blog/2021-09-30-ensemble-analysis_files/figure-html/plot-forecasts-1.svg-- the newly generated version has a very different time scale on the x-axis (presumably because the maximum date was not specified in the code that produces it), and it may no longer convey the same meaning and detail as was originally intended. There could be more examples of this among the other images, but i did not inspect them all. -
Not rebuilding [all of] the pages would also let us keep the
2020-08-28-api.htmlfile intact, which preserves the quidel data table that becomes inaccessible with a rebuild. I dont think that we should be too concerned with trying to remove the table from our site, since it has already been captured by the wayback machine. -
The generated stuff includes changes to
js&cssfiles which may be important for dependencies or style updates, but i'm not sure what the ramifications are.
-
-
Most or all of it is hidden now, but do we want to show the "API key"-related code? That makes it easier for users to replicate the results we showcase in the blog posts, it may encourage more users to sign up for and use API keys, and it can provide references for users who may run into API key problems in the future. This point becomes moot if we are going to preserve the "old" snapshots of the blog posts.
We probably want input from @rlunde on the questions i raised above.
This reverts commit d21f9a9.
Co-authored-by: melange396 <[email protected]>
Co-authored-by: melange396 <[email protected]>
Co-authored-by: melange396 <[email protected]>
Co-authored-by: melange396 <[email protected]>
Co-authored-by: melange396 <[email protected]>
Co-authored-by: melange396 <[email protected]>
Co-authored-by: melange396 <[email protected]>
|
I like the idea of showing how the API key is used. Also, only updating blog posts that have code changes is a good idea - thanks! Do you know if we're going to have problems with it trying to add recreated markdown / html assets in the future, for blog posts that are not changed? |
|
@melange396 So for the API key related code, I have a few questions. Right now api-related code is written as sth like this |
Yeah, thats a great idea. I would use the python/rlang comment convention so it looks something like this: options(covidcast.auth = Sys.getenv("API_KEY")) # for more on API keys, see: https://cmu-delphi.github.io/delphi-epidata/api/api_keys.html |
|
@minhkhul @melange396 I've lost track of where we are with this one. Does Minh's last commit address all of the changes George asked for, or do we still need to figure out whether the template blog is still visible? |
|
@rlunde @melange396 So I couldn't figure out exactly how the template-post hiding mechanism works, in that it historically has shown up in netlify preview, but is not seen in our released site. But in previous blog posts releases, I added new blog posts without making any changes related to the template post. So I don't think there needs to be any changes to that template posts now for it to be hidden. It's just that I have been looking around the repo for the past 15 minutes and could not find exactly how template-post is hidden. But it is. |
|
@melange396 So regarding the template file, the template rmd file is marked as a draft. Netlify previews are configured to build drafts while our actual deployment does not. |
Good find! Might we want to change the 'deploy-preview' build command so our Netlify preview actually reflects what will be pushed live? Or if we dont wanna go that far, we should at least add to the comment on the |
This reverts commit 17a1150.
…API key + instruction link code line in that chunk
| @@ -195,7 +196,7 @@ <h2>CLI-in-Community</h2> | |||
| <p>In both plots, we see a reassuring trend, | |||
| but the trend on the left is noticeably stronger. | |||
| Indeed, the correlation here between the Google signal and case rates is | |||
| 0.84, | |||
| 0.83, | |||
There was a problem hiding this comment.
this isnt a huge change, but it is kind of concerning... i think we can probably sweep it under the rug for now, unless @rlunde feels like escalating it.
| @@ -114,7 +115,7 @@ <h2>Travel and Other Social Behaviors During US Holidays</h2> | |||
| filter(geo_value %in% str_to_lower(statelist)) %>% | |||
| mutate(value = plyr::mapvalues(geo_value, str_to_lower(statelist), regions), | |||
| value = as.integer(factor(value))) | |||
|
|
|||
| attr(regionmap, "metadata") <- list(geo_type = "state") | |||
There was a problem hiding this comment.
this is weird: it was added in https://github.com/cmu-delphi/www-main/pull/706/files#diff-1fa4417df3994a017fb9ca08ed2a579d658b7167ccdae44b295a9e3aa3bf0880R111 but hasnt shown up til now... i guess we havent actually re-rendered this page in over a year? [no action needed; just wanted to point this out for posterity]
This PR fixes 2 main problems with blog build + misc api key related improvements as following:
Fix #887
Fix Quidel data references in blog post by:
This approach preserves the content of the blog post while satisfying our goal to not show quidel data.
Fix covidcast versioning in python chunk
Problem: A blog post was not building due to lacking api key in a python chunk in the .Rmd. The deeper problem was the python covidcast version was locked at 0.1.3 since 3 years ago (not due to a need for that particular version), so the error messages were not helpful.
Solution: Add api key to post & upgrade covidcast version to latest, which fixed the problem.
Misc improvements:
Make references to API key invisible in old blog posts.Make references to API key visible in old blog posts + instruction on how to get an API key for readers.