Not require foreignKey.reference.resource for self-referencing

[roll]

• fixes Allow omitting foreignKey.reference.resource for self-referencing specs#878

---

Rationale

Please see the linked issue

[cloudflare-pages]

Deploying datapackage with    Cloudflare Pages

Latest commit:	8a18a48
Status:	✅  Deploy successful!
Preview URL:	https://5cc2e3f9.datapackage.pages.dev
Branch Preview URL:	https://878-simplify-fk-self-referen.datapackage.pages.dev

View logs

[peterdesmet]

Why not always require resource? Even if it is self-referencing.

[roll]

@peterdesmet
In my opinion, requiring resource: "" is quite weird and error-prone (empty string is falsy in majority of languages). At least it would make mor sense for me if it were something like resource: "self"

[peterdesmet]

I was suggesting to have the resource name, even for an internal relationship. So for a resource called events:

"foreignKeys": [
    {
      "fields": "parentEventID",
      "reference": {
        "resource": "events", <-- Not "" or "self", just the resource name
        "fields": "eventID"
      }
    }
  ]

[roll]

@peterdesmet
That's pretty eloquent. I would support this

[khusmann]

I really like @peterdesmet 's self-referencing approach, but what do we do if it is a standalone schema definition (i.e. defined in a json outside of a datapackage)? We could encounter the situation where two resources of different names to point to the same external schema...

That said, I think this is more an argument against external schemas, rather than this approach to foreignKey.reference.resource. I've never liked externally loaded schemas because it means it a web request is potentially required to get the field structure of a resource definition to parse local data... (and the remote definition could change or go offline, rendering the local data unusable!) I think this issue with foreignKey.reference.resource points to another example of why it'd good to require schemas to be statically bundled with resource definitions, rather than externally linked.

[roll]

VETOED by WG

I'll create an updated PR based on @peterdesmet's solution

[roll]

Let's have one last try here according to this comment - #37 (comment)

[peterdesmet]

Let's have one last try here according to this comment - #37 (comment)

I'm sorry to reopen the discussion. 😄 Can you adapt this PR? The spec indeed removes the requirement, but the text currently uses "self".

[roll]

@peterdesmet
Oops I used the same branch. Thanks! It's been fixed now

[roll]

@peterdesmet
Can you please re-vote if I got you right 😃

[roll]

@pschumm
@pwalsh
@nichtich
Can you please take a look at this PR?

[peterdesmet]

@roll I approve this PR.

[roll]

Thanks @peterdesmet !

[pschumm]

Looks good to me.

[roll]

Thanks!

We miss one vote now. @khughitt @PietrH would you consider supporting it?

[khughitt]

Change looks good to me!

[roll]

Thanks!

ACCEPTED by WG (6/9)