trustfall icon indicating copy to clipboard operation
trustfall copied to clipboard

WIP: Add documentation for the `Adapter` trait

Open ginger51011 opened this issue 2 years ago • 6 comments

I think it's important that this trait in particular is well-documented. Right now I'm working based on demos, but I chose to relate it to GraphQL since it's currently the query language of choice.

I might be a bit off with descriptions here, and I think the query_hint and vertex_hint may need more explanations, since they are ignored in the demos from what I can see. But according to #131 these are subject to change soonTM :small_airplane:

ginger51011 avatar Feb 10 '23 12:02 ginger51011

Tbh I'm thinking of completely revamping this trait.

#131 will remove the explicit "hint" arguments in favor of a &QueryInfo value. If we then adopt the (in my mind, better) nomenclature from BasicAdapter, then there's basically nothing left here that won't get changed.

What do you think? Are the names of BasicAdapter better and easier to understand? If we were to adopt them here in a breaking change, would that be an improvement?

obi1kenobi avatar Feb 10 '23 15:02 obi1kenobi

Sorry about the CI, have added "editor.formatOnSave": true now...

I have tried, based on demo-hackernews, to provide adequate examples and explanations for some of the adapter methods. However, I might be off at some places. But perhaps it is a good idea for a complete newbie to attempt to write the docs, because it makes it painfully obvious what is hard to understand...

--- Above this line was before you posted your comment :)

Right now I think this trait is quite difficult to understand, but looking at BasicAdaptor I realize that I should have looked there first!

If nothing else, BasicAdapter has a lot better method names, and just that makes it easier to use. It might be worth a breaking change, because right now the demos just ignore query_hint and vertex_hint, which makes them basically magic.

For me at least, with very limited background in graph theory and graph databases, but some knowledge of GraphQL, I think it might be helpful to explain concepts in the adaptors by describing them in a GraphQL context (like "a starting vertex is like a field of the Query type in a GraphQL schema"), like I have tried to do in this PR.

ginger51011 avatar Feb 10 '23 15:02 ginger51011

Right now I think this trait is quite difficult to understand, but looking at BasicAdaptor I realize that I should have looked there first!

This is the fault of insufficient docs 😅 Let's try to figure out how to best direct the next person to that easier-to-implement trait. It's easier to implement because it can't do everything that the "real" one under the hood can do, but it's enough for most cases.

If nothing else, BasicAdapter has a lot better method names, and just that makes it easier to use. It might be worth a breaking change, because right now the demos just ignore query_hint and vertex_hint, which makes them basically magic.

Yes, agreed. I realized a design like #131 is necessary because the hint parameters are technically all you need but in practice are just waaay too tedious to use. After I've put in a few more iterations on #131, I'd love your help and feedback there on naming and documentation. It's super useful to have a second pair of eyes on these things — thanks for taking the time!

I think it might be helpful to explain concepts in the adaptors by describing them in a GraphQL context (like "a starting vertex is like a field of the Query type in a GraphQL schema"), like I have tried to do in this PR.

I understand the sentiment here. I have two concerns, and a concrete proposal which I'll name first.

I think we may be better off with an "Learning Trustfall for GraphQL users" page somewhere in the docs, instead of inlining the GraphQL analogies and comparisons into the main documentation.

Here's why:

  • Trustfall isn't actually GraphQL: it uses an analogous syntax for the most part, but its semantics are quite different and more powerful. When seeing the syntax, people very familiar with GraphQL are already tempted to (incorrectly) say "ah, this is just another GraphQL federation implementation" -- the briefest of looks at the HackerNews thread on Trustfall is sufficient to confirm this. We need to discourage that leap as hard as we can, rather than encouraging it by comparing and analogizing to GraphQL across the docs.
  • People not familiar with GraphQL shouldn't have to learn it first before understanding our docs. Trustfall needs to be friendly to people from a SQL background, or Excel background, or dataframes background. Explaining Trustfall concepts by comparing them with analogous GraphQL concepts makes GraphQL a "preferred" background for learning Trustfall, which not only doesn't help everyone else but actively hurts: "oh no, another thing I have to learn before I can understand this."

By having documentation free of concepts from other languages, and having separate "Learning Trustfall for X users" pages in the docs, we can let the readers self-select into the right groups and then we can make the right analogies to them:

  • In the SQL case: "The @optional directive is like a SQL LEFT JOIN: it means it's okay if the data on the other side doesn't exist. In that case, any selections from inside the @optional block have a value of null."
  • In the GraphQL case: "A starting vertex is one available as a field on the Query type of the schema." etc.

What do you think?

obi1kenobi avatar Feb 10 '23 15:02 obi1kenobi

Yes, agreed. I realized a design like https://github.com/obi1kenobi/trustfall/pull/131 is necessary because the hint parameters are technically all you need but in practice are just waaay too tedious to use. After I've put in a few more iterations on https://github.com/obi1kenobi/trustfall/pull/131, I'd love your help and feedback there on naming and documentation. It's super useful to have a second pair of eyes on these things — thanks for taking the time!

I'd be happy to help! Right now I'm looking intro using Trustfall myself, so I'm very interested in seeing improvements.

I think we may be better off with an "Learning Trustfall for GraphQL users" page somewhere in the docs, instead of inlining the GraphQL analogies and comparisons into the main documentation.

Reading your response I would say I agree. I think the reason I have to reason around it like that myself is that when there is nothing to latch onto, it is much easier to map it to something you already know.

If anything, I think the docs (or README, or whatever) could use some images of operations. I saw some on your blog, but they don't explain in much detail what actually happens inside the adapters/engine, and how that would be different from GraphQL. This might be just me though.

Another question is: How much does one want to steer the adapter implementations? Looking at the demos, there are definitely patterns that occurs several times. Are all users to write their own implement_property! when most of them look the same? Could there be room for a trustfall_adapter_macros (or is there one that I have missed?)

  • In the SQL case: "The @optional directive is like a SQL LEFT JOIN: it means it's okay if the data on the other side doesn't exist. In that case, any selections from inside the @optional block have a value of null."
  • In the GraphQL case: "A starting vertex is one available as a field on the Query type of the schema."

Just those two are very useful: Take something the users know, and explain new concept with old ones. Perhaps we shouldn't explain things using only GraphQL, but implementing these in the docs (where people go first) might come a long way.

ginger51011 avatar Feb 10 '23 16:02 ginger51011

If anything, I think the docs (or README, or whatever) could use some images of operations. I saw some on your blog, but they don't explain in much detail what actually happens inside the adapters/engine, and how that would be different from GraphQL. This might be just me though.

The README definitely needs several layers' worth of rework. Some images would definitely be nice, but I think some of the "getting started" docs might be more useful now that many people are already at least somewhat familiar with the project and what it does.

Ultimately it's just a question of sequencing the work in maximum-impact order. I think good images are a bit difficult to make, and probably not the immediate highest priority compared to "getting started" docs that point to BasicAdapter instead of Adapter, for example.

Another question is: How much does one want to steer the adapter implementations? Looking at the demos, there are definitely patterns that occurs several times. Are all users to write their own implement_property! when most of them look the same? Could there be room for a trustfall_adapter_macros (or is there one that I have missed?)

I've started several drafts of macros, and never quite found a design I loved. It's really unfortunate that macros are not allowed to generate a "match arm," because that might have been nice.

I'll continue thinking about this. If you end up looking at the problem and find a possible design something you like, please bring it up in an issue or draft PR!

obi1kenobi avatar Feb 10 '23 18:02 obi1kenobi

I'd also love to know about the domain where you're thinking about using Trustfall, if you'd be up for a chat? I hang out on the rust-lang Zulip and you can also find me on Discord if you'd prefer that. No pressure, obviously.

obi1kenobi avatar Feb 10 '23 18:02 obi1kenobi