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(example): example extended with fabric #640

Merged

Conversation

jordigiam
Copy link

Resolve #362
Depends on #616
Depends on #634

@github-actions
Copy link

🎉 Great news! Looks like all the dependencies have been resolved:

💡 To add or remove a dependency please update this issue/PR description.

Brought to you by Dependent Issues (:robot: ). Happy coding!

@AzaharaC AzaharaC force-pushed the feature/extend-example-fabric branch from a3ca0a4 to bb2b44a Compare March 15, 2021 13:09
@jordigiam jordigiam marked this pull request as ready for review March 15, 2021 14:51
takeutak
takeutak previously approved these changes Mar 15, 2021
@jordigiam jordigiam force-pushed the feature/extend-example-fabric branch 5 times, most recently from 000512a to d75234d Compare March 18, 2021 14:21
petermetz
petermetz previously approved these changes Mar 19, 2021
@petermetz
Copy link
Contributor

@jordigiam CI is failing due to it running out of disk space[1] which I remedied successfully in the past by adding this to the test where it gives out, same thing as here:
packages/cactus-plugin-ledger-connector-fabric/src/test/typescript/integration/fabric-v2-2-x/run-transaction-endpoint-v1.test.ts

  // Always set to true when GitHub Actions is running the workflow.
  // You can use this variable to differentiate when tests are being run locally or by GitHub Actions.
  // @see https://docs.github.com/en/actions/reference/environment-variables
  if (process.env.GITHUB_ACTIONS === "true") {
    // Github Actions started to run out of disk space recently so we have this
    // hack here to attempt to free up disk space when running inside a VM of
    // the CI system.
    await Containers.pruneDockerResources();
  }

[1]:

ail out! packages/cactus-plugin-ledger-connector-fabric/src/test/typescript/integration/fabric-v1-4-x/run-transaction-endpoint-v1.test.ts
internal/fs/utils.js:220
    throw err;
    ^

Error: ENOSPC: no space left on device, write
    at Object.writeSync (fs.js:581:3)
    at Object.writeFileSync (fs.js:1275:26)
    at NYC.writeProcessIndex (/home/runner/work/cactus/cactus/node_modules/nyc/index.js:490:8)
    at /home/runner/work/cactus/cactus/node_modules/nyc/bin/nyc.js:68:9
    at ChildProcess.<anonymous> (/home/runner/work/cactus/cactus/node_modules/foreground-child/index.js:52:5)
    at ChildProcess.emit (events.js:210:5)
    at ChildProcess.EventEmitter.emit (domain.js:476:20)
    at maybeClose (internal/child_process.js:1021:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:283:5) {
  errno: -28,
  syscall: 'write',
  code: 'ENOSPC'
}

takeutak
takeutak previously approved these changes Mar 23, 2021
@AzaharaC AzaharaC dismissed stale reviews from takeutak and petermetz via 10ed775 March 24, 2021 12:41
@AzaharaC AzaharaC force-pushed the feature/extend-example-fabric branch from d75234d to 10ed775 Compare March 24, 2021 12:41
@AzaharaC AzaharaC requested review from petermetz and takeutak March 24, 2021 15:04
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.

@jordigiam @AzaharaC Looks legit.

TLDR:
I made some fixes/chores/docs changes, is it okay to force push? Didn't want to accidentally erase your work.

Story:
When testing locally, the docker image build wasn't working for me because I broke it in a PR of mine a few weeks back (corda connector) so I added a hotfix for that in a commit that I recommend to keep separate from the main PR commit (so the PR would have 2 commits which is fine in this case IMO because I did not want to complicate things even further by opening a separate PR with the hot-fix and then have this one wait for that and be forced to depend on multiple parent branches, etc...)

The other thing I did was publishing the container image to DockerHub under a new tag and updated the READMEs to use that image so that once we merge this PR, everyone will see the latest example in the root readme with Fabric included.

I have all the above on a branch on my work because I did not want to risk force pushing onto this and then erasing something that you were working on (but if you confirm that it's cool then I'll go ahead and perform the force push).

https://github.com/petermetz/cactus/tree/feature/extend-example-fabric

@AzaharaC
Copy link
Contributor

@jordigiam @AzaharaC Looks legit.

TLDR:
I made some fixes/chores/docs changes, is it okay to force push? Didn't want to accidentally erase your work.

Story:
When testing locally, the docker image build wasn't working for me because I broke it in a PR of mine a few weeks back (corda connector) so I added a hotfix for that in a commit that I recommend to keep separate from the main PR commit (so the PR would have 2 commits which is fine in this case IMO because I did not want to complicate things even further by opening a separate PR with the hot-fix and then have this one wait for that and be forced to depend on multiple parent branches, etc...)

The other thing I did was publishing the container image to DockerHub under a new tag and updated the READMEs to use that image so that once we merge this PR, everyone will see the latest example in the root readme with Fabric included.

I have all the above on a branch on my work because I did not want to risk force pushing onto this and then erasing something that you were working on (but if you confirm that it's cool then I'll go ahead and perform the force push).

https://github.com/petermetz/cactus/tree/feature/extend-example-fabric

@petermetz I have been reviewing your changes and I think that the push can be done without problems, so do it.

Why though? Because if you are building the project during the
building of the supply chain app example container image, then
the "generate-server" script of the corda connector was failing
due to a running docker daemon not being present in the image
at build time (even with the docker-in-docker base image because
that also only has a docker daemon running at runtime of the
container not at build time).

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@petermetz petermetz force-pushed the feature/extend-example-fabric branch from 10ed775 to 02af31c Compare March 25, 2021 16:12
Signed-off-by: jordigiam <jordi.giron.amezcua@accenture.com>
@petermetz petermetz force-pushed the feature/extend-example-fabric branch from 02af31c to a96ba92 Compare March 25, 2021 16:14
@petermetz petermetz self-requested a review March 25, 2021 16:14
@petermetz
Copy link
Contributor

@takeutak Review was dismissed because of conflict resolution, could you please approve again? Thank you in advance!

@petermetz petermetz merged commit 55d6587 into hyperledger-cacti:main Mar 29, 2021
@petermetz petermetz deleted the feature/extend-example-fabric branch March 29, 2021 20:11
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.

docs(examples): extend supply chain app with fabric specific elements
4 participants