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

Update 'Running a Fabric Application' tutorial for Fabric Gateway #3110

Merged
merged 1 commit into from
Dec 16, 2021

Conversation

bestbeforetoday
Copy link
Member

@bestbeforetoday bestbeforetoday commented Dec 9, 2021

Also fixed a few build warnings in other documents.

Contributes to #2807

.. code:: bash
The sample application uses an X.509 certificate for the Org1 user identity, and a signing implementation based on the
corresponding private key. These credentials are created using the Org1 Certificate Authority when the test network
is started.
Copy link
Contributor

Choose a reason for hiding this comment

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

So needs something like "The following code snippet ... " is what, or does what.

The terminal output entry should look similar to below:
An initialization function to set up initial ledger state would typically be called only once after a smart contract
is deployed for the first time. A new version of the smart contract would likely not require the initialization to be
performed again. In this case it is good practice for the transaction function to check and perhaps also record some
Copy link
Contributor

Choose a reason for hiding this comment

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

tx function being the smart contract code, within the chaincode right.

them to bytes internally.

Next, the sample application submits a transaction to transfer ownership of the newly created asset. This time
the transaction is invoked using ``submitAsync()``, which returns after successfully submitting the endorsed
Copy link
Contributor

Choose a reason for hiding this comment

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

note first time "endorsed" mentioned; may need to add it earlier.

Terminal Output:

.. code:: bash
The final part of the sequence demonstrates an error submitting a transaction. In this example, the application attempts
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mention "the gateway service" here as a player.

docs/source/write_first_app.rst Outdated Show resolved Hide resolved
docs/source/write_first_app.rst Outdated Show resolved Hide resolved
docs/source/write_first_app.rst Outdated Show resolved Hide resolved
docs/source/write_first_app.rst Outdated Show resolved Hide resolved
docs/source/write_first_app.rst Outdated Show resolved Hide resolved
docs/source/write_first_app.rst Outdated Show resolved Hide resolved
to find a set of required endorsing peers for the chaincode, invoke the chaincode
on the required number of peers, gather the chaincode endorsed results from those peers,
and finally submit the transaction to the ordering service.
1. Endorse the transaction proposal.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to explain this at a high level (one paragraph), even though it is described in other key concepts topics in more detail.
e.g. something at this level of detail "submitTransaction() will request the targeted Gateway peer to execute the chaincode and gather endorsements from the required number of peers to meet the endorsement policy (in our case the Org1 gateway peer as well as the Org2 peer), then will sign the transaction."

Copy link
Member Author

Choose a reason for hiding this comment

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

It was described at a high level previously but was deemed confusing, so avoided.

Copy link
Contributor

Choose a reason for hiding this comment

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

@joshhus what was the concern? A sentence or two would definitely help here. Feel free to propose an edit to what I suggested.

Copy link
Contributor

@joshhus joshhus Dec 15, 2021

Choose a reason for hiding this comment

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

The discussion here was that it initially read as if an "endorsement" by one peer / org then leads to a commitment to the ledger. Without a full understanding of endorsement and endorsement policies, that sounded potentially misleading to me. We discussed this when reviewing the other topics also - endorsement by one org does not mean the Tx passes endorsement (policies). So generally a semantics issue of being imprecise with the word "endorsed" in different contexts.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually the comments before were on whether the brief description of executing the transaction on peers matched the description of the endorsement process we already have in other pages.

My intent with this list of steps is to signpost the following application flow, which is what gets executed when you call submitTransaction(), not to describe the transaction flow at the server side:

Understanding the application API flow is important for interpreting error types that may be generated by that flow, touched on in the last step.

docs/source/write_first_app.rst Outdated Show resolved Hide resolved
docs/source/write_first_app.rst Outdated Show resolved Hide resolved
docs/source/write_first_app.rst Outdated Show resolved Hide resolved
@joshhus
Copy link
Contributor

joshhus commented Dec 15, 2021

The discussion around "gateway peer" - I don't see the comment here any more - was only that "gateway" to modify peer was deemed redundant in this context (in our prior reviews), because all peers are now "gateway" peers. So not to avoid using "peer" but to avoid using "gateway peer". ... So in prior docs we use "peer" when that is important - such as in getting a connection from the client app. ... When it's the "Fabric Gateway service" specifically (running on the peer) that is doing things, then we call it that, as opposed to a "peer" doing things.

@bestbeforetoday
Copy link
Member Author

bestbeforetoday commented Dec 15, 2021

The discussion around "gateway peer" - I don't see the comment here any more - was only that "gateway" to modify peer was deemed redundant in this context (in our prior reviews), because all peers are now "gateway" peers. So not to avoid using "peer" but to avoid using "gateway peer". ... So in prior docs we use "peer" when that is important - such as in getting a connection from the client app. ... When it's the "Fabric Gateway service" specifically (running on the peer) that is doing things, then we call it that, as opposed to a "peer" doing things.

The client app only interacts with the Gateway service. Whether that Gateway service runs on a peer is irrelevant to the client application. There was a point in time where the Gateway service was implemented in a separate component, not in a peer, and this could be the case again in the future. It really makes no difference from the client application perspective discussed in this tutorial.

@joshhus
Copy link
Contributor

joshhus commented Dec 16, 2021

Maybe 3 sections would have been clearer. To consider another time without a deadline --
A. Here's how to do the tutorial, only. (task)
B. Here is the Tx flow and other concepts that are revealed in the tutorial. (conceptual). If you want to read about that.
C. Here are the overall Fabric Gateway topics (links) and fabric topics (links) that apply to the tutorial (reference). If you want to go read that. ... (Note reference could also be presented as prereq, before the task.)

@denyeart
Copy link
Contributor

The discussion around "gateway peer" - I don't see the comment here any more - was only that "gateway" to modify peer was deemed redundant in this context (in our prior reviews), because all peers are now "gateway" peers. So not to avoid using "peer" but to avoid using "gateway peer". ... So in prior docs we use "peer" when that is important - such as in getting a connection from the client app. ... When it's the "Fabric Gateway service" specifically (running on the peer) that is doing things, then we call it that, as opposed to a "peer" doing things.

The client app only interacts with the Gateway service. Whether that Gateway service runs on a peer is irrelevant to the client application. There was a point in time where the Gateway service was implemented in a separate component, not in a peer, and this could be the case again in the future. It really makes no difference from the client application perspective discussed in this tutorial.

I beg to differ... the client needs to connect to a certain endpoint, and that endpoint is generally known as the peer endpoint throughout all the other doc topics. I'm just suggesting to mention that the gateway service runs in a peer so that readers know exactly which endpoint they are connecting to. That seems like a fundamental concept that client application developers need to know. Yes, the Gateway could in theory run anywhere, and that's exactly why the reader may be in doubt. But we chose to run it in the peer and should communicate that. Just one mention early on would be sufficient to clear up any doubt.

@bestbeforetoday
Copy link
Member Author

I beg to differ... the client needs to connect to a certain endpoint, and that endpoint is generally known as the peer endpoint throughout all the other doc topics. I'm just suggesting to mention that the gateway service runs in a peer so that readers know exactly which endpoint they are connecting to. That seems like a fundamental concept that client application developers need to know. Yes, the Gateway could in theory run anywhere, and that's exactly why the reader may be in doubt. But we chose to run it in the peer and should communicate that. Just one mention early on would be sufficient to clear up any doubt.

The code snippets and some of the supporting text already explicitly state that it's the peer endpoint. I'll add a little more wording to reinforce that though.

Also fixed a few build warnings in other documents.

Signed-off-by: Mark S. Lewis <mark_lewis@uk.ibm.com>
@denyeart denyeart merged commit 7bd3c9e into hyperledger:main Dec 16, 2021
@denyeart
Copy link
Contributor

@Mergifyio backport release-2.4

@mergify
Copy link

mergify bot commented Dec 16, 2021

backport release-2.4

✅ Backports have been created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants