Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Remove Redis dependencies #1014

Closed
wants to merge 2 commits into from
Closed

Conversation

igor-egorov
Copy link
Contributor

@igor-egorov igor-egorov commented Feb 27, 2018

Description of the Change

Redis dependencies were removed from docker-compose files, configuration files and all the rest places where Redis was mentioned.

Benefits

Completely removes Redis from the project after hyperledger-iroha/iroha#933

@@ -81,6 +71,4 @@
POSTGRES_PORT: 5432
POSTGRES_USER: "{{ postgresUser }}"
POSTGRES_PASSWORD: "{{ postgresPassword }}"
REDIS_HOST: "{{ redisName }}"
REDIS_PORT: 6379
KEY: node{{ key }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add end line. Also, configure up your IDE with auto-insertion of end line


By local environment, it is meant to have daemon process and the components (Redis and Postgres) deployed without any containers. This might be helpful in cases when messing up with Docker is not preferred — generally a quick exploration of the features.
By local environment, it is meant to have daemon process and the components (Postgres) deployed without any containers. This might be helpful in cases when messing up with Docker is not preferred — generally a quick exploration of the features.
Copy link
Contributor

Choose a reason for hiding this comment

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

and the components (Postgres) deployed

Maybe rework it with

...and Postgres deployed...

@@ -43,7 +38,7 @@ In case of valid assumptions, the only thing that remains is to launch the daemo
+---------------+-----------------------------------------------------------------+
| Parameter | Meaning |
+---------------+-----------------------------------------------------------------+
| config | configuration file, containing postgres, and redis connection, |
| config | configuration file, containing postgres, |
| | and values to tune the system |
Copy link
Contributor

Choose a reason for hiding this comment

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

If it possible fit this to the previous line.

@igor-egorov igor-egorov force-pushed the IR-952-remove-redis-deps branch 2 times, most recently from d5ec5a2 to 173ae6f Compare February 27, 2018 13:59
@@ -43,8 +38,8 @@ In case of valid assumptions, the only thing that remains is to launch the daemo
+---------------+-----------------------------------------------------------------+
| Parameter | Meaning |
+---------------+-----------------------------------------------------------------+
| config | configuration file, containing postgres, and redis connection, |
| | and values to tune the system |
| config | configuration file, containing postgres connection, and values |
Copy link
Contributor

Choose a reason for hiding this comment

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

extra comma after connection

@@ -77,23 +72,11 @@ Then, you have to create an enviroment for the image to run without problems:
Create docker network
"""""""""""""""""""""

Containers for Redis, Postgres, and Iroha should run in the same virtual network, in order to be available to each other. Create a network, by typing following command (you can use any name for the network, but in the example, we use *iroha-network* name):
Containers for Postgres, and Iroha should run in the same virtual network, in order to be available to each other. Create a network, by typing following command (you can use any name for the network, but in the example, we use *iroha-network* name):
Copy link
Contributor

Choose a reason for hiding this comment

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

extra comma after Postgres

@@ -122,7 +105,7 @@ Running iroha daemon in docker container
""""""""""""""""""""""""""""""""""""""""

There is a list of assumptions which you should review before proceeding:
* Postgres and redis servers are running on the same docker network
* Postgres server are running on the same docker network
Copy link
Contributor

Choose a reason for hiding this comment

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

server is running

@@ -1,7 +1,7 @@
#!/usr/bin/env bash

#
# This simple script starts redis and postgres docker containers,
# This simple script starts postgres docker containers,
Copy link
Contributor

Choose a reason for hiding this comment

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

postgres docker container

Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
@codecov
Copy link

codecov bot commented Feb 27, 2018

Codecov Report

Merging #1014 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1014   +/-   ##
========================================
  Coverage    33.06%   33.06%           
========================================
  Files          524      524           
  Lines        15990    15990           
  Branches     10182    10182           
========================================
  Hits          5287     5287           
  Misses        1371     1371           
  Partials      9332     9332

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 9f5a13e...60b192b. Read the comment docs.

Copy link
Contributor

@lebdron lebdron left a comment

Choose a reason for hiding this comment

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

It would also be great to fix comments in postgres_block_query.hpp, because pull request descriptions states:

all the rest places where Redis was mentioned

@neewy
Copy link
Contributor

neewy commented Feb 28, 2018

For the sake of complete corrections maybe iroha-api modification is required.

Signed-off-by: Igor Egorov <igor@soramitsu.co.jp>
@igor-egorov
Copy link
Contributor Author

Corrections suggested by @lebdron have been applied in 60b192b

@sorabot
Copy link

sorabot commented Feb 28, 2018

SonarQube analysis reported 5 issues

  1. MINOR postgres_block_query.cpp#L144: Define a constant instead of duplicating this literal " AND height = " 2 times. rule
  2. MINOR postgres_block_query.hpp#L72: Unused private function: 'PostgresBlockQuery::getBlockIds' rule
  3. MINOR postgres_block_query.hpp#L80: Unused private function: 'PostgresBlockQuery::getBlockId' rule
  4. MINOR postgres_block_query.hpp#L90: Unused private function: 'PostgresBlockQuery::callback' rule
  5. INFO postgres_block_query.hpp#L44: Undocumented API: PostgresBlockQuery rule

@neewy
Copy link
Contributor

neewy commented Feb 28, 2018

please use feature prefix for new changes as stated here https://github.com/hyperledger/iroha/wiki/Iroha-working-agreement#243-feature

@igor-egorov
Copy link
Contributor Author

igor-egorov commented Mar 1, 2018

@neewy, thanks for the link! I have taken this into account. Since I have to rename the branch, this PR will be automatically closed and another will be issued instead.

I have prepared complementary PR for iroha-api repository - hyperledger-iroha/iroha-api#58

@igor-egorov igor-egorov closed this Mar 1, 2018
@igor-egorov igor-egorov deleted the IR-952-remove-redis-deps branch March 1, 2018 07:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants