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

refactor(indy-testnet) improve setup and tools of indy-testnet #1813

Merged
merged 2 commits into from
Feb 9, 2022

Conversation

outSH
Copy link
Contributor

@outSH outSH commented Jan 26, 2022

This is a step before moving discounted-cartrade into container.

I've divided this PR into two commits for clarity. First one changes the logic and code, second is reorganization of different indy-testnet components (I've moved them around a little). If they were in single commit it'd be hard to follow the changes, I think :)

Documentation for components affected by this commit is outdated now, I'll update it after I finish my changes on them.

Depends on #1800

Depends on #1806

refactor(indy-testnet) improve setup and tools of indy-testnet

  • req_discounted_cartrade.py - reuse proof when possible, add new flags to force new proof generation and to ignore discounted-cartrade request to simplify tests, put this tool into container.
  • testsock.js - fix formatting, add missing package, use new validator address (nginx).
  • Refactor all indy-testnet Dockerfiles according to docker best practices, reduce container size and complexity.
  • Use nginx as a proxy to validator (as documented).

refactor(indy-testnet): organize indy-testnet components

  • Move and adjust indy validator Dockerfile from tools/docker/indy-testnet to it's sources in packages-python/cactus_validator_socketio_indy.
  • Rename indy validator dir from cactus_validator_socketio to cactus_validator_socketio_indy.
    • @izuru0 @petermetz What naming convention should be used for python packages? Is proposed name OK (it's based on current naming), or maybe we should use naming from packages/, i.e. cactus-plugin-ledger-connector-indy-socketio
  • Rename clientbase image to indy-sdk-cli, move it to separate dir in tools/docker.
  • Adjust paths in indy-testnet docker-compose, add dependency to start nginx after validator.
  • Commit empty dir tools/docker/indy-testnet/indy_pool/sandbox directly to repo, remove redundant setup script.

@outSH
Copy link
Contributor Author

outSH commented Jan 26, 2022

@petermetz @takeutak @izuru0 Please review (only last two commits)

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.

Rename indy validator dir from cactus_validator_socketio to cactus_validator_socketio_indy.
@izuru0 @petermetz What naming convention should be used for python packages? Is proposed name OK (it's based on current naming), or maybe we should use naming from packages/, i.e. cactus-plugin-ledger-connector-indy-socketio

@outSH I would try to do away with the socketio suffixes in all the packages for both python or npm and instead look to name them based on the real differentiating features.

@outSH
Copy link
Contributor Author

outSH commented Jan 31, 2022

I would try to do away with the socketio suffixes in all the packages for both python or npm and instead look to name them based on the real differentiating features.

Ok, thank you

Copy link
Contributor

@izuru0 izuru0 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot removed the dependent label Feb 9, 2022
@github-actions
Copy link

github-actions bot commented Feb 9, 2022

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.

@outSH LGTM, just going to ask that you please rebase and squash as needed because the dependent PRs have been merged already so the ideal end state is that those commits no longer show up on the list of commits for this PR if the rebase was done. As always do let me know if I can help with any of the git stuff and we can figure it out.

req_discounted_cartrade.py - reuse proof when possible, add new flags to
force new proof generation and to ignore discounted-cartrade request to
simplify tests, put this tool into container. testsock.js - fix
formatting, add missing package, use new validator address (nginx).
Refactor all indy-testnet docerfiles according to docker best practices,
reduce container size and complexity. Use nginx as a proxy to validator
(as documented).

Closes: hyperledger-cacti#1812
Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
Move and adjust indy validator Dockerfile from tools/docker/indy-testnet
to it's sources in packages-python/cactus_validator_socketio_indy.
Rename indy validator dir from cactus_validator_socketio to
cactus_validator_socketio_indy. Rename clientbase image to indy-sdk-cli,
move it to separate dir in tools/docker. Adjust paths in indy-testnet
docker-compose, add dependency to start nginx after validator. Commit
empty dir tools/docker/indy-testnet/indy_pool/sandbox directly to repo,
remove redundant setup script.

Closes: hyperledger-cacti#1812
Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
@outSH
Copy link
Contributor Author

outSH commented Feb 9, 2022

LGTM, just going to ask that you please rebase and squash

@petermetz Yes, of course, already done ;)

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.

@outSH Perfect, thank you very much!

@petermetz petermetz merged commit dd443b5 into hyperledger-cacti:main Feb 9, 2022
@outSH outSH deleted the indy_docker_refactor branch March 21, 2022 16:26
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