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

feat(validator): Indy validator and indy-testnet files #1099

Merged
merged 1 commit into from
Jul 14, 2021

Conversation

izuru0
Copy link
Contributor

@izuru0 izuru0 commented Jun 28, 2021

Added new code that talks to indy pool. It submits request to indy it received and sends response back.
The python code will be updated in the future so that users can simply pip install them and run (although it is incomplete at the moment). The directory structure reflects this intention.

Also included are updated version of indy-testnet files. The code described above needs this environment to properly run.

@takeutak takeutak changed the title Indy validator and indy-testnet files feat(validator): Indy validator and indy-testnet files Jun 28, 2021
@takeutak
Copy link
Contributor

takeutak commented Jun 28, 2021

This PR resolves the issue #959 and #866 . LGTM.

As your PR seems to intend, this PR includes python Validator codes then its codes are put on the different directory from /packages (in order to avoid the influence from the ci checking tool for TypeScript). I agreed with the arrangement of codes.

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.

@izuru0 Thank you for the contribution! Could you add a remote adapter for this in Typescript so that the API server can pull it in as a plugin as well?

@izuru0
Copy link
Contributor Author

izuru0 commented Jun 30, 2021

@petermetz Thank you for your comment.

It is nice to have interoperability between API server and connector (possibly not just for Indy but other ledgers too).

I think we can design a wrapper to make indy validator (in python) to be callable, but I suspect that it could take some time to design a standard interface between API server in typescript and (arbitrary) connector in python.

For that reason, can I ask this code to be accepted for the time being, and consider the designing/implementing such interface as other task?

@petermetz
Copy link
Contributor

@petermetz Thank you for your comment.

It is nice to have interoperability between API server and connector (possibly not just for Indy but other ledgers too).

I think we can design a wrapper to make indy validator (in python) to be callable, but I suspect that it could take some time to design a standard interface between API server in typescript and (arbitrary) connector in python.

For that reason, can I ask this code to be accepted for the time being, and consider the designing/implementing such interface as other task?

@izuru0 Sure thing.

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.

@izuru0 Please squash into a single commit, then it LGTM.
Let me know if you need help with the git mechanics, happy to help.

@takeutak
Copy link
Contributor

@izuru0 It seems that he created a new pull request #1117 having the same codes instead of squashing. So I think it is no problem to close this PR #1099. Is it OK?

@petermetz So would you mind if you review the new PR #1117 again?

@petermetz
Copy link
Contributor

@izuru0 It seems that he created a new pull request #1117 having the same codes instead of squashing. So I think it is no problem to close this PR #1099. Is it OK?

@petermetz So would you mind if you review the new PR #1117 again?

@takeutak @izuru0 Any chance that you could just do the squash instead of me reviewing the entire PR again? It takes 30 seconds to squash, if you don't know how to do it I can help.

Signed-off-by: Izuru Sato <sato.izuru@fujitsu.com>
@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2021

Codecov Report

Merging #1099 (f0c60ee) into main (6d1de27) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1099   +/-   ##
=======================================
  Coverage   73.14%   73.14%           
=======================================
  Files         246      246           
  Lines        8617     8617           
  Branches      996      996           
=======================================
  Hits         6303     6303           
  Misses       1783     1783           
  Partials      531      531           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d1de27...f0c60ee. Read the comment docs.

@takeutak takeutak merged commit 8eef3fa into hyperledger-cacti:main Jul 14, 2021
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.

4 participants