Remote data #494

RealOrangeOne · 2018-10-20T21:21:10Z

Fixes #476.
Re-implements #465
Extends #379

Allow loading data from remote endpoints, and parsing through the existing formats load_data supports.

This PR is intentionally designed to reuse as much of the code as possible when reading from data files or remote endpoints. This not only reduces the chance of odd bugs appearing, but means I don't have to write as much! Because I also moved things out into separate files, GitHub makes it looks like i've done way more work than I actually have.

Support requesting remote data
Allow loading data as plain text, with no parsing
Implement a cache to reduce repeat API calls / parsing
Move data loading into separate file to split it out, it's quite a lot of code!
Send the current version of zola in the user agent. There's no reason for this, I just thought it might be nice!
Rename kind argument format, as it's more appropriate
If someone tries to load files from outside their site directory, throw an error. This should also include symlinks

It would be possible to edit the headers when sending / receiving a response with the format argument. But i decided not to do that, and we / someone else can add it further down the line.

None of this code has been run through rustfmt, intentionally.

Note: Currently the test can_load_remote_data is failing, when it shouldn't be. This will fix itself once the repo move occurs.

cc @kellpossible, enjoy!

URL type

RealOrangeOne · 2018-10-20T21:21:42Z

(Netlify is failing because it can't find the new function, which is to be expected)

components/templates/src/lib.rs

components/templates/Cargo.toml

components/templates/src/global_fns/load_data.rs

Keats · 2018-10-22T19:38:11Z

components/templates/src/global_fns/load_data.rs

+            "toml" => load_toml(data),
+            "csv" => load_csv(data),
+            "json" => load_json(data),
+            "plain" => to_value(data).map_err(|e| e.into()),


I'm not sure about plain, what is it even loading?

plain is just the format which returns the data raw, with no additional processing. Either the request body or the file contents.

but when is it needed? I can't really think of a usecase where you would want that

There probably are some use cases out there, and seeing as it was basically free to implement, thought I might as well. It's probably something which may be critical to someone out there

Just a heads up - I actually would make use of plain to load CSS and shove it inline at build-time, or to load a base64-encoded value that's ugly in code but needs to exist. They're rare use cases but they do exist.

components/templates/src/global_fns/load_data.rs

components/utils/src/fs.rs

docs/content/documentation/templates/overview.md

docs/templates/index.html

… remote-data

RealOrangeOne · 2018-10-24T17:19:42Z

@Keats I've made all the requested changes, and a couple bits of cleanup.

Keats

A couple more comments but looking good!

Cargo.toml

Keats · 2018-10-25T13:35:49Z

components/templates/src/global_fns/load_data.rs

+enum OutputFormat {
+    TOML,
+    JSON,
+    CSV,


I wouldn't uppercase them even if they are acronyms to be consistent with Rust style (https://doc.rust-lang.org/1.0.0/style/style/naming/README.html)

Keats · 2018-10-25T13:36:11Z

components/templates/src/global_fns/load_data.rs

+    TOML,
+    JSON,
+    CSV,
+    Plain


I think I would still remove that for now, it can be added later if necessary

Keats · 2018-10-25T13:37:00Z

components/templates/src/global_fns/load_data.rs

+}
+
+impl OutputFormat {
+    fn from(output_format: String) -> Result<Self> {


you could impl FromStr instead

I did think of that, but because it's possible for it to fail (ie the string doesnt match one of the enum values), it's unsafe, and TryFrom is only in nightly right now.

FromStr returns a Result though? FromStr and TryFrom look to be identical for this usecase

Aha, I found my confusion. I was looking at From rather than FromStr. Yes you're right, FromStr would be ideal to implement here!

Keats · 2018-10-25T13:39:04Z

components/templates/src/global_fns/load_data.rs

+    }
+}
+
+fn get_cache_key(data_source: &DataSource, format: &OutputFormat) -> u64 {


Making this part of the impl of DataSource would be cleaner imo

I was toying with where to put it, as it's also equally valid on OutputFormat, but as most of the computation varys for DataSource, I guess there makes the most sense of the 2.

Keats · 2018-10-25T13:41:51Z

docs/content/documentation/templates/overview.md

+{{ response }}
+```
+
+By default, the response body will be returned with no parsing. This can be changed by using the `format` argument as above.


Hmm I guess we need plain because of that? How about making the format argument mandatory?

I could make it mandatory, but the fact it bases it off the file extension is a really nice touch for local data files. Having to specify that my data.json file is a JSON file feels unnecessary when it's so easy to guess from the file extension, and change it by default if required.

I'm not sure myself what is the best choice there. I kind of like the nice UX for ending in JSON but in practice for remote data you will need to specify the format so it feels a bit like it could surprise the end user when something works for local data but not remote.

I would tend towards removing the special casing but not 100% convinced.

Keats · 2018-10-29T11:10:24Z

The 2 failing tests need to be fixed but I can merge it after

We cant use a fixed hash because it's based on the absolute path, which changes for each dev (probably)

RealOrangeOne · 2018-10-29T12:50:05Z

I've fixed the tests, and they run fine locally, however seem to be doing odd things on Travis.

Keats · 2018-10-29T13:12:49Z

For global_fns::load_data::tests::fails_when_request_404s, you can make up a random URL to avoid issues like that.
For the other, you can use a fake public api like https://jsonplaceholder.typicode.com/todos/1 to avoid 403 and various issues

This means tests can run on window / *nix

RealOrangeOne · 2018-10-29T14:25:53Z

Tests passing 🎉 (besides netlify, but that doesn't look related to this PR?)

Keats · 2018-10-29T14:30:33Z

Netlify is not going to work for a while with Zola sadly, not related to your PR :)

Thanks!

kellpossible · 2018-10-30T03:57:14Z

Nice one @RealOrangeOne !!

RealOrangeOne added 17 commits October 19, 2018 09:09

Require EITHER path or url argument

ed41e37

Refactor global fns into module for future expansion

d7ae694

Abstract load_data to separate file

5992ecd

Change builtin functions to take preloaded data, and lay foundations for

730eec9

URL type

Automatically determine file kind again if the passed arg is a file

ad9c0bc

Store path argument as PathBuf

8031228

Prevent directory traversal when loading data files

fcb8d5a

Calculate a key ready for the cache

9cf0bb6

Implement cache

3c1f3df

Request data from remote

6447c51

Validate URLs

829c769

Bake version into user agent

be1f321

Better handle directory traversal issues, and missing file errors

8f227e8

Update documentation for load_data

d418a3b

Change kind argument to format

e130075

Handle 404s better

084607d

Restore accidentally commented out constant

d2ce779

Keats requested changes Oct 22, 2018

View reviewed changes

RealOrangeOne added 11 commits October 23, 2018 09:07

Remove clap dependency and hardcode package name

44720b4

Remove unnecessary xnor

2f15f11

Remove unnecessary intermediary variable

6529d04

Remove debug log

bec826c

Fix typo in docs

0725959

Remove data fetch from docs templates

f8b3adb

Rename ProvidedArgument enum so it makes more sense

5b003f6

Merge branch 'remote-data' of github.com:RealOrangeOne/gutenberg into…

6e44a7c

… remote-data

Use enum for output format!

56a8949

Use std::hash::Hash for hashing rather than MD5

4649302

Add accept header to request

a6a1b9f

impl arg unpack into DataSource enum

db1a98f

Keats requested changes Oct 25, 2018

View reviewed changes

RealOrangeOne added 4 commits October 25, 2018 15:34

Remove notice about bumping templates version

c7b0cc7

Rename enum to match Rust conventions

cb8944b

Don't custom implement FromStr

5d5294a

Move cache key to DataSource, and implement Hash

dd00c51

RealOrangeOne added 3 commits October 29, 2018 11:58

Fix path tests

079d743

We cant use a fixed hash because it's based on the absolute path, which changes for each dev (probably)

Replace url with one which exists now

5a67446

Remove unused import

d0931a5

RealOrangeOne added 2 commits October 29, 2018 13:35

Use httpbin as demp urls

a396b04

Don't mention path split character in tests

f1365eb

This means tests can run on window / *nix

Keats merged commit aad12d8 into getzola:next Oct 29, 2018

RealOrangeOne deleted the remote-data branch October 29, 2018 20:19

erlend-sh mentioned this pull request Nov 21, 2018

Load data from remote endpoints #476

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remote data #494

Remote data #494

RealOrangeOne commented Oct 20, 2018 •

edited

RealOrangeOne commented Oct 20, 2018

Keats Oct 22, 2018

RealOrangeOne Oct 23, 2018

Keats Oct 23, 2018

RealOrangeOne Oct 24, 2018

ryanmcgrath Jan 3, 2019

RealOrangeOne commented Oct 24, 2018

Keats left a comment

Keats Oct 25, 2018

Keats Oct 25, 2018

Keats Oct 25, 2018

RealOrangeOne Oct 25, 2018

Keats Oct 25, 2018

RealOrangeOne Oct 25, 2018

Keats Oct 25, 2018

RealOrangeOne Oct 25, 2018

Keats Oct 25, 2018

RealOrangeOne Oct 25, 2018

Keats Oct 26, 2018

Keats commented Oct 29, 2018

RealOrangeOne commented Oct 29, 2018

Keats commented Oct 29, 2018

RealOrangeOne commented Oct 29, 2018

Keats commented Oct 29, 2018

kellpossible commented Oct 30, 2018

Remote data #494

Remote data #494

Conversation

RealOrangeOne commented Oct 20, 2018 • edited

RealOrangeOne commented Oct 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RealOrangeOne commented Oct 24, 2018

Keats left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keats commented Oct 29, 2018

RealOrangeOne commented Oct 29, 2018

Keats commented Oct 29, 2018

RealOrangeOne commented Oct 29, 2018

Keats commented Oct 29, 2018

kellpossible commented Oct 30, 2018

RealOrangeOne commented Oct 20, 2018 •

edited