-
Notifications
You must be signed in to change notification settings - Fork 404
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
Added official Iroha Library package compatible with Iroha 1.0 beta-2 #35
Added official Iroha Library package compatible with Iroha 1.0 beta-2 #35
Conversation
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
… in the second round Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
iroha_node0: | ||
image: hyperledger/iroha-docker@sha256:b82a24eee7cc4215bf70d9f79acab8b817e4cf36cc9f87c1c63202dca552cf0f | ||
iroha-node0: | ||
image: hyperledger/iroha:x86_64-develop-latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We (Iroha team) plan to change Iroha's docker naming in the nearest future, so now this PR uses image hyperledger/iroha:x86_64-develop-latest
without any sha256.
When we rename it I'll do commit in this branch with the new name of container on dockerhub.
Error occurred when installing iroha-lib, any idea??
|
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
@stinger112 , can you help solve the problem?? |
@haojun please try to use iroha-lib 0.0.6. It seems required ABI in 0.0.5 is missing. |
I'm sorry for late response. |
Please also apply this diff for genesis block and txCounter fixes https://gist.github.com/victordrobny/02cbeeed165bb7226e67f57e8712c6a0 |
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
@haojun please review and approve so that we can use Caliper for our CI routine :) |
Dear @haojun sorry for this mess up! We no need to merge this PR because I have to wait new release version of Iroha. So I suggest to wait when new version will be released (it's very very soon) and fix library and Iroha container on such version. |
@stinger112 OK, got it |
@haojun @stinger112 let's merge since we have released? Arsenii, please confirm and fix conflicts |
@neewy Great! And please run eslint to check if the code follows current linting rules (remove iroha folder from the .eslintignore file and run |
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
48ca935
to
98921ea
Compare
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
@haojun, I updated the code. |
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
A node-pre-gyp error happened when installing grpc@1.12.2, any idea how to fix it?
|
I can install Caliper with dependencies from scratch in empty docker containers. Checked systems: ubuntu:18.04 So, I can't reproduce the bug. Please check your environment. |
package-lock.json
Outdated
@@ -0,0 +1,4627 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should not be commited.
@@ -24,7 +24,8 @@ | |||
"commander": "^2.11.0", | |||
"dockerode": "^2.5.0", | |||
"fs-extra": "^4.0.2", | |||
"grpc": "1.10.1", | |||
"grpc": "^1.12.2", | |||
"iroha-lib": "^0.1.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- what's the purpose of add npx?
- Is grpc > 1.12 MUST be used for iroha. If not, please keep the version as 1.10.1 (we found the new version of grpc is somehow incompatible with fabric-sdk )
- This package is for common use, so please don't add iroha's dependencies here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- According to the philosophy of NPM all apps should be self-contained. If I have no globally installed ESLint or version of my installed ESLint another than specified in the package for some reason (e.g. in a case of CI),
npm test
andnpm run-script lint
may have strange behavior or even don't work at all.
So I add npx as script wrapper to return such functionality.
If you want to I can replace it with directly calling./node_modules/.bin/eslint .
or even return it to the previous state. - grpc must not be used by Iroha, I returned it to the 1.10.1
- IMPORTANT! We definitely need to add some mechanism to fix versions of used connector packages for different ledgers in order to ensure the stability of the current version of Caliper.
Exactly like we did use docker images and hashes/tags.
I think just description in Readme not enough to solve this issue, therefore I decided to fix version in the common package.json until the future improvements.
I know this not the best solution, but it is something.
If you have any ideas how to gracefully solve this problem, it would be nice!
By the way, is grpc in package.json not a ledger-related dependency too?
@stinger112 |
src/iroha/iroha.js
Outdated
*/ | ||
|
||
/* eslint-disable no-console */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now you can use log function from src/comm/util.js to resolve the eslint error instead of disable it.
We may extend this function later.
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
Signed-off-by: Arseniy Fokin <stinger112@gmail.com>
7b8b017
to
537a082
Compare
@haojun Please add the following line in the package.json
It has not been pushed, I forget! |
@stinger112 Got it. I didn't notice it either. |
Updare simple scenario test configs
Description
Iroha contributors have created NPM package with Iroha Connector Library, so this PR adds iroha-lib into Caliper and update Iroha's docker container to latest develop version.