search

p2panda / handbook

Website with tutorials, specification, info and learn sections of p2panda
https://p2panda.org
Creative Commons Attribution Share Alike 4.0 International
87 stars 8 forks source link

Aquadoggo tutorial needs a section to copy the config file #286

Closed jadehopepunk closed 1 year ago

jadehopepunk commented 1 year ago

Hi, I'm a new user and working through the tutorials from scratch, on a fresh machine.

I found that following the tutorial as it stood I got mysterious errors logged:

[2023-08-05T02:13:12Z DEBUG aquadoggo::node] Schema not added to schema provider: not supported schema_definition_v1
[2023-08-05T02:13:12Z DEBUG aquadoggo::node] Schema not added to schema provider: not supported schema_field_definition_v1

The node also didn't support the query for all_schemas which is used as the example in this tutorial to prove everything was working. And finally, it was impossible to start on the next tutorial and create schemas.

The solution is to copy the example config file. This PR pops that into the tutorial, before the run stage (to avoid asking them to copy the config and then restart the node).

Feel free to completely rewrite, but as a new user this was a hard thing to spot, I noticed the addition of the config file by searching recent PRs.

sandreae commented 1 year ago

Hi @craigambrose, thanks for creating this PR. Your analysis of the issue and proposed solution is spot-on, really sorry we didn't get around to updating all the tutorials yet, there has been some fairly fast paced development happening in the last weeks which we're only just emerging from.

As you found, one of the recent changes was the introduction of a config.toml file where the schema a node supports is configured. The current behaviour when this file isn't present is for the node to not support any schema. There is of course an alternative default behaviour, which is for the node to replicate all schema when no config is found. We considered both and settled on the former.

I'm now wondering if the latter behaviour is actually preferable though, especially from the perspective of new developers being able to start playing with a node nice and easily. I'm going to work on the tutorials today and decide about this in the process. If you have any thoughts yourself (as a developer new p2panda) let me know :smile:

In any case thanks for this PR, depending on the decision about the above I'll either merge as-is or update for any changes to the default behaviour.

sandreae commented 1 year ago

Hi @craigambrose, I went ahead changed the default behaviour around supported schema in aquadoggo as described above, both me and @adzialocha think it's a better approach (https://github.com/p2panda/aquadoggo/pull/487). I'll hold off merging till tomorrow in case you have any comments.

I've also made updates to all tutorials and associated repositories, so you should be able to follow through these without issue (using the optional-config-file aquadoggo branch for now).

sandreae commented 1 year ago

Once https://github.com/p2panda/aquadoggo/pull/487 is merged into main in aquadoggo it makes the changes in this PR redundant so I'll close this too. Thanks for raising the issue though, and reminding us to check all the tutorials :man_facepalming:

jadehopepunk commented 1 year ago

Thanks @sandreae, I think your solution is much better. Optional config file, and optional schema whitelist, sounds like a great idea. That'll help people get up and running fast, but still give the option to tune nodes to handle unwanted traffic in production later.

sandreae commented 1 year ago

Great! Glad you agree 👍 I'll merge that PR to aquadoggo main then.