Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define resolution function with data types, property values, and simple metadata structures #298

Conversation

jricher
Copy link
Contributor

@jricher jricher commented May 27, 2020

This adapts the DID Resolution functional definition defined by #253 into a single typed function with requirements for the function signature and implementation conformance. Property values for input and output metadata structures are defined here, with pointers to the DID registry for management of properties. Requirements for when and how to return each value are also added.

Simple metadata structures are defined here, without detail for conformance or transmission.

This builds on #295, #296, and #297.


💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on May 27, 2020, 9:28 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Spec Generator - Spec Generator is the web service used to build specs that rely on ReSpec.

🔗 Related URL


😭  Sorry, there was an error generating the HTML. Please report this issue!
�[36mSpecification: https://rawcdn.githack.com/jricher/did-core/1b2f1523e079a2c09f7dad531aa87b132c6092cb/index.html?isPreview=true&publishDate=2020-05-27�[39m
�[36mReSpec version: 25.6.0�[39m
�[36mFile a bug: https://github.com/w3c/respec/�[39m
�[36mError: Error: Evaluation failed: Timeout: document.respecIsReady didn't resolve in 19587ms.�[39m
�[36m    at ExecutionContext._evaluateInternal (/u/spec-generator/node_modules/puppeteer/lib/ExecutionContext.js:122:13)�[39m
�[36m    at runMicrotasks (<anonymous>)�[39m
�[36m    at processTicksAndRejections (internal/process/task_queues.js:97:5)�[39m
�[36m    at async ExecutionContext.evaluate (/u/spec-generator/node_modules/puppeteer/lib/ExecutionContext.js:48:12)�[39m
�[36m    at async generateHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:128:12)�[39m
�[36m    at async fetchAndWrite (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:95:18)�[39m
�[36m    at async Object.generate [as respec] (/u/spec-generator/generators/respec.js:14:20)�[39m
�[36m    at async generate (/u/spec-generator/server.js:91:29)�[39m
�[36m  -- ASYNC --�[39m
�[36m    at ExecutionContext.<anonymous> (/u/spec-generator/node_modules/puppeteer/lib/helper.js:111:15)�[39m
�[36m    at DOMWorld.evaluate (/u/spec-generator/node_modules/puppeteer/lib/DOMWorld.js:112:20)�[39m
�[36m    at runMicrotasks (<anonymous>)�[39m
�[36m    at processTicksAndRejections (internal/process/task_queues.js:97:5)�[39m
�[36m  -- ASYNC --�[39m
�[36m    at Frame.<anonymous> (/u/spec-generator/node_modules/puppeteer/lib/helper.js:111:15)�[39m
�[36m    at Page.evaluate (/u/spec-generator/node_modules/puppeteer/lib/Page.js:860:43)�[39m
�[36m    at Page.<anonymous> (/u/spec-generator/node_modules/puppeteer/lib/helper.js:112:23)�[39m
�[36m    at generateHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:128:23)�[39m
�[36m    at runMicrotasks (<anonymous>)�[39m
�[36m    at processTicksAndRejections (internal/process/task_queues.js:97:5)�[39m
�[36m    at async fetchAndWrite (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:95:18)�[39m
�[36m    at async Object.generate [as respec] (/u/spec-generator/generators/respec.js:14:20)�[39m
�[36m    at async generate (/u/spec-generator/server.js:91:29)�[39m
�[36m�[39m

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

conform to valid syntax. (See <a href="#did-syntax"></a>.)
</dd>
<dt>
unauthorized
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting... I would expect this code to be surfaced by an http / transport, but not by a functional interface.

This feels like transport logic leaking into the contract interface, it also seems to leak some privacy information, I would rather just get a 404 from the method resolver for such cases, but I may not have interpreted this correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is just one of many possible error conditions that the function could return. I thought of a couple that seemed obvious and common, but if this one is potentially controversial than it can be removed from this PR set. It's more important that there be a place to list and return error values that the group can then expand as use cases require them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you covered this on the call, and as long as the working group understands that git is not append only, and we can and will refactor, i see no reason to block this PR...

this <a>DID resolver</a>.
</dd>
<dt>
not-found
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exceedingly common, eager to see use this.

Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about unauthorized, but the rest of this seems good to me.

I'd suggest removing unauthorized error for now or adding a link to an issue so we can discuss further without cluttering the PR.

@brentzundel brentzundel added contract metadata issues and PRs related to metadata labels Jun 5, 2020
@msporny msporny added the do not merge Do not merge - waiting on resolution to issue label Jun 25, 2020
@msporny
Copy link
Member

msporny commented Jul 3, 2020

PR #331 has been merged. @jricher, is there any part of this PR that needs to be applied to a metadata PR or can we close this PR now?

@msporny msporny closed this Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Do not merge - waiting on resolution to issue metadata issues and PRs related to metadata
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants