Skip to content

Conversation

@jcortejoso
Copy link
Contributor

Description

Refactor to use individual compose files for each component.

Is this PR related with an open issue?

Related to Issue #49, #25

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Follows the code style of this project.
  • Tests Cover Changes
  • Documentation

Funny gif

Put a link of a funny gif inside the parenthesis-->

@jcortejoso jcortejoso requested review from H34D and aaitor November 30, 2018 15:31
@jcortejoso jcortejoso requested a review from ssallam November 30, 2018 15:40
Copy link

@ttmc ttmc left a comment

Choose a reason for hiding this comment

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

This refactoring makes a lot of sense. I took a quick look and commented on a couple of things. I will try out these compose files in the coming week.

- keeper-contracts
environment:
KEEPER_HOST: keeper-contracts
OCEAN_HOST: aquarius
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no KEEPER_HOST and OCEAN_HOST anymore, variables need to target localhost since browser is not in docker network. If variables are not set then they default to localhost anyway so these lines are not needed actually
NODE_HOST: localhost
AQUARIUS_HOST: localhost
BRIZO_HOST: localhost

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I understand. So the browser connect directly to the components.

start_ocean.sh Outdated
COMPOSE_FILES=${COMPOSE_FILES/ -f ${COMPOSE_DIR}\/pleuston.yml/}
printf $COLOR_Y'Starting without Pleuston...\n\n'$COLOR_RESET
;;
--local-parity-node)
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this starting? a new network or a node that connects to the testnet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends the variable KEEPER_NETWORK_NAME. With this refactor the start_ocean.sh should work exactly the same as before the refactor (there is not any functionality change).
That line in particular remove the pleuston.yml docker-compose in the running sequence.

@ttmc
Copy link

ttmc commented Dec 5, 2018

Because OCEAN_VERSION now defaults to latest, we don't need to have the following code in start_ocean.sh any more... but since the --latest option has been in use, we also don't want to break existing workflows and documentation. Maybe what we could do instead of removing the --latest option is tell the user that latest is the default now, so they don't need to include the --latest option any more. Over time, people will remove it and the docs will catch up, and then we can remove it for good.

         --latest)
             export OCEAN_VERSION=latest
             printf $COLOR_Y'Switched to latest components...\n\n'$COLOR_RESET
             ;;

@jernejpregelj
Copy link
Contributor

Don't remove the --latest. If we make one stable release and start again with development process with development unstable builds we will need it again. Docker-images are the best way if you want to start all the services so we should offer them as stable and as a development version.

@H34D
Copy link
Contributor

H34D commented Dec 5, 2018

please see: #74

@jcortejoso jcortejoso mentioned this pull request Dec 5, 2018
6 tasks
@H34D
Copy link
Contributor

H34D commented Dec 6, 2018

let me write some documentation today, also signing in secret store does not work as expected

@ttmc
Copy link

ttmc commented Dec 6, 2018

I tested ./start_ocean.sh --no-pleuston --local-pond-node just now and most things seemed to work okay, but I did notice this:

brizo_1                      | [2018-12-06 09:18:17 +0000] [10] [INFO] Starting gunicorn 19.9.0
brizo_1                      | [2018-12-06 09:18:17 +0000] [10] [INFO] Listening at: http://0.0.0.0:8030 (10)
brizo_1                      | [2018-12-06 09:18:17 +0000] [10] [INFO] Using worker: sync
brizo_1                      | [2018-12-06 09:18:17 +0000] [12] [INFO] Booting worker with pid: 12
ocean_secret-store-cors-proxy_1 exited with code 1
brizo_1                      | 2018-12-06 09:18:17 612e26a0194b root[12] INFO Logging configuration loaded from file: logging.yaml
brizo_1                      | [2018-12-06 09:18:17 +0000] [12] [ERROR] Exception in worker process
brizo_1                      | Traceback (most recent call last):
brizo_1                      |   File "/usr/local/lib/python3.6/site-packages/gunicorn/arbiter.py", line 583, in spawn_worker
brizo_1                      |     worker.init_process()
brizo_1                      |   File "/usr/local/lib/python3.6/site-packages/gunicorn/workers/base.py", line 129, in init_process
...
brizo_1                      |     contract_name,
brizo_1                      |   File "/usr/local/lib/python3.6/site-packages/squid_py/keeper/utils.py", line 37, in get_contract_by_name
brizo_1                      |     raise FileNotFoundError('Keeper contract {} file not found: {}'.format(contract_name, path))
brizo_1                      | FileNotFoundError: Keeper contract OceanMarket file not found: /usr/local/artifacts/OceanMarket.ocean_poa_net_local.json
brizo_1                      | [2018-12-06 09:18:17 +0000] [12] [INFO] Worker exiting (pid: 12)
keeper-node_1                | 2018-12-06 09:18:18 UTC Public node URL: enode://20b27cf57493cd641b2fe5df6e39c0d52a96bd0e4f3416fcd35d6fcab6e8c76748e9b18fa579a2306e587c79e6bcd5b646c2021dea413b3b7ad98366594c5347@172.15.0.12:30303
brizo_1                      | [2018-12-06 09:18:18 +0000] [10] [INFO] Shutting down: Master
brizo_1                      | [2018-12-06 09:18:18 +0000] [10] [INFO] Reason: Worker failed to boot.

@H34D
Copy link
Contributor

H34D commented Dec 6, 2018

yep there are three bugs right now:

  • secret store signing node does not sign properly
  • brizo is not working with anything else than --local-lake-node
  • --local-pond-node does not mine transactions

i am working on that

Copy link
Contributor

@ssallam ssallam left a comment

Choose a reason for hiding this comment

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

LGTM

@H34D H34D merged commit 2b610fe into develop Dec 10, 2018
@H34D H34D deleted the feature/refactor branch December 10, 2018 12:32
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.

7 participants