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

docs(whitepaper): add a section of APIs and communication protocols #179

Merged
merged 4 commits into from
Jun 25, 2020
Merged

docs(whitepaper): add a section of APIs and communication protocols #179

merged 4 commits into from
Jun 25, 2020

Conversation

takeutak
Copy link
Contributor

This pull-request is mainly modified from PR#160 , but slightly changed from that PR (newly adding an API table of Verifier and Validator)

Signed-off-by: Takuma TAKEUCHI takeuchi.takuma@fujitsu.com

@takeutak
Copy link
Contributor Author

Note that this pull-request is linked with the issue-list #178

@petermetz
Copy link
Contributor

Thanks for your comment, Peter! I made a new pull-reflect reflecting your comment #179
Then I close this old pull-request.

@takeutak Thank you! Did you see my other comments though? There were maybe 5 or 6, can't remember the exact number, not just the one about the table of contents.

@takeutak
Copy link
Contributor Author

Thanks for your comment, Peter! I fixed not only the contents table number but the section numbers.
If there is another formal problem, I would like to push my another pull-request after this pull request is approved.

@petermetz
Copy link
Contributor

Thanks for your comment, Peter! I fixed not only the contents table number but the section numbers.
If there is another formal problem, I would like to push my another pull-request after this pull request is approved.

@takeutak These are the comments I meant during the call, they are visible if you navigate to the old #160 PR and scroll down a bit.
This is how it looks when I go to #160 but it may be presented in some different format on your side because you are the PR author, but I'm not sure to be honest.

For reference, see screenshot below:

image

@takeutak
Copy link
Contributor Author

I fixed the whitepaper changing /v1/... to /bl-api/vi/... reflecting your advice, Peter.
"bl-api" means "business logic plugin's api" since only "api" does not specify what type of API.

If you had written other comments on my old pull-request #160 , would you mind if you paste them in the new pull-request?
I cannot see your comments on my old pull-request #160 .

@petermetz
Copy link
Contributor

I fixed the whitepaper changing /v1/... to /bl-api/vi/... reflecting your advice, Peter.
"bl-api" means "business logic plugin's api" since only "api" does not specify what type of API.

Any chance that it could be /api/v1/bl/.../ instead?

Reasoning: I'm advocating for using the greater to smaller hierarchical organization of the path elements so that what's on the left is a super-set of what's on the right when comparing any adjacent path elements. In other words: All APIs are APIs, some of them might be bl or some-other-type-of-api but they are both APIs anyway so the /api/ is the super-set for both either way.
In practice all this boils down to the fact that calling it /bl-api/... breaks the path matching logic that assumes that all APIs are APIs by doing a runtime check on an HTTP requests path such as req.path.startsWith("/api/").
I know this doesn't seem like a big deal because you could just say that now we check it like

const isApiReq = req.path.startsWith("/api/") || req.path.startsWith("/bl-api/");

Which would of course work, but it is increasing the number of if conditions (implicit or explicit, doesn't matter) in the code linearly scaling up with the number of different APIs that we will invent in the future (which I expect to be unbounded given our design principles strongly favoring extensibility/flexibility/pluggability/etc.)

In general, I'm always out on a quest to eliminate if conditions in code when possible because they are slippery slopes leading to arrow programming in it's worst, but even when used moderately they are increasing the number of valid execution paths the code can take exponentially since if conditions open up separate branches of the decision tree as the code execution if progressing. (of course there are justified uses of if conditions since without them most software would be pretty useless, but you get the point)
It's impossible either way to prove correctness of any non-trivial software because of the combinatoric explosion, but that doesn't mean we shouldn't strive to keep the number of possible states/branches to a minimum because it also makes it much easier for humans to digest the code and what it does.
One of my beliefs is that every non-trivial code-base that is actively being worked is constantly on the edge of a cliff above the canyon of unmaintainable code (some call this technical debt, but I don't like finance terminology creeping into software engineering which I consider to be closer to a form of art than economic sciences)

I know this is getting super abstract and kinda off-topic, but I wanted to make sure I explain my thoughts on this in detail because whenever I talk about things like this I'm always self-conscious about it looking like dumb hair splitting/preaching dogmas (like who cares about two extra characters in the request path, right?).

If you had written other comments on my old pull-request #160 , would you mind if you paste them in the new pull-request?
I cannot see your comments on my old pull-request #160 .

Yeah I had like 5 or 6 of them.
That's pretty annoying that reviews just disintegrate when a PR is closed, especially because some PRs I've seen were re-opened 3-4 times so shoveling over all reviews every time to each new PR that gets opened is going to be a pain and something that will scale very poorly as the community/PR count grows.
So, I opened a support request with Github, hoping they'll say something that's helpful in providing you with the review comments without me having to re-do all of them.

In the meantime, here are all the comments of mine from the previous PR (Github support might take a while, their auto response said they de-prioritize tickets because of the virus and their reduced staff).

image

@takeutak
Copy link
Contributor Author

takeutak commented Jun 23, 2020

As Peter's comment on Cactus meeting, I fixed my pull-request (from /bl-api/v1/ to /api/v1/bl/ )
Would you mind if you confirm this pull-request, Peter?

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jonathan-m-hamilton jonathan-m-hamilton left a comment

Choose a reason for hiding this comment

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

LGTM!

@petermetz
Copy link
Contributor

petermetz commented Jun 25, 2020

Oops, there's a conflict in the table of contents because of the other whitepaper PR I just merged in a minute ago.
@takeutak I'll update the TOC and push a change with that onto your branch (just the TOC, rest of the content won't be touched) so that the merge conflict is resolved and we can merge the PR.

@petermetz petermetz added the documentation Improvements or additions to documentation label Jun 25, 2020
takeutak added 4 commits June 25, 2020 10:09
Signed-off-by: Takuma TAKEUCHI <takeuchi.takuma@fujitsu.com>
…ommunication protocols

Signed-off-by: Takuma TAKEUCHI <takeuchi.takuma@fujitsu.com>
…ion protocols

Signed-off-by: Takuma TAKEUCHI <takeuchi.takuma@fujitsu.com>
…tocols

Signed-off-by: Takuma TAKEUCHI <takeuchi.takuma@fujitsu.com>
@petermetz petermetz merged commit 92e7b0b into hyperledger-cacti:master Jun 25, 2020
@takeutak takeutak deleted the docs_whitepaper_add_a_section_of_APIs_and_communication_protocol branch September 1, 2020 02:55
ryjones pushed a commit that referenced this pull request Feb 1, 2023
HTLC Support added for Corda
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants