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: added redis in docker-compose #3107

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sahil9001
Copy link

@sahil9001 sahil9001 commented Oct 16, 2024

Description:
This PR adds redis to the docker-compose so that both servers can share the cache among themselves.

Screenshot 2024-10-19 at 4 20 29 PM

Related issue(s):

Fixes #1869

Notes for reviewer:

Docker Containers up & running :
Screenshot 2024-10-23 at 11 43 12 PM

Environmental variables :
Screenshot 2024-10-23 at 11 44 17 PM
Screenshot 2024-10-23 at 11 44 29 PM
Screenshot 2024-10-23 at 11 44 44 PM

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@sahil9001 sahil9001 requested a review from a team as a code owner October 16, 2024 10:57
Copy link
Collaborator

@AlfredoG87 AlfredoG87 left a comment

Choose a reason for hiding this comment

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

Overall looks good.
leaved some nits.

docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
@AlfredoG87
Copy link
Collaborator

AlfredoG87 commented Oct 16, 2024

@sahil9001 |
Also for contributing to ANY hedera (hiero) repository, you will need to sign your commits... there are several great guides on how to configure your keys and sign-off your commits.

when you do so the DCO check on the CI will pass.

hint: you will need to force push, once you configure your GPG Key do the following:

git reset --soft HEAD~1
git commit -s -m "Your commit message"
git push --force

The git reset the last commit but keeps the changes (soft)
the git commit with -s signs off the commit.
the git push force will overwrite the branch.

👍

@sahil9001 sahil9001 force-pushed the add-redis-in-docker-compose branch 4 times, most recently from 349acb1 to 9f82371 Compare October 19, 2024 12:10
@quiet-node quiet-node added good first issue Good for newcomers hacktoberfest Issues shown by lists for the Hacktoberfest and made for newcomers to do the first contribution. Internal For changes that affect the project's internal workings but not its outward-facing functionality. labels Oct 22, 2024
@quiet-node quiet-node added this to the 0.59.0 milestone Oct 22, 2024
@sahil9001
Copy link
Author

@AlfredoG87 can you check here? I don't have the option to ask for re-review :/

Copy link
Collaborator

@AlfredoG87 AlfredoG87 left a comment

Choose a reason for hiding this comment

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

Looks good, just 2 nits.

PS: it would be cool if you can add some screenshots of the docker containers running, and its env context when running the docker compose with this change. You can put these screenshots on the PR Description under "Notes for reviewer" section.

docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
@sahil9001 sahil9001 requested a review from a team as a code owner October 23, 2024 17:46
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.72%. Comparing base (7796aca) to head (bada091).
Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3107      +/-   ##
==========================================
- Coverage   83.16%   79.72%   -3.45%     
==========================================
  Files          63       65       +2     
  Lines        4236     4232       -4     
  Branches      830      840      +10     
==========================================
- Hits         3523     3374     -149     
- Misses        470      577     +107     
- Partials      243      281      +38     
Flag Coverage Δ
config-service 100.00% <ø> (?)
relay 85.30% <ø> (-0.29%) ⬇️
server ?
ws-server ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 44 files with indirect coverage changes

@Nana-EC
Copy link
Collaborator

Nana-EC commented Oct 23, 2024

Hey @sahil9001 , thanks for your contribution, this is great.
You have some issues on your signing.
Please follow instructions here to ensure you've signed off.
All commits needs to be signed and signed off :)
More details in our Contributing Guide

Signed-off-by: Sahil Silare <sahilsilare@gmail.com>
@sahil9001
Copy link
Author

Hey @sahil9001 , thanks for your contribution, this is great. You have some issues on your signing. Please follow instructions here to ensure you've signed off. All commits needs to be signed and signed off :) More details in our Contributing Guide

done buddy, thanks for this. I learnt this new thing today!

@AlfredoG87
Copy link
Collaborator

Don't forget to add a blank line at the end of the file. 👍

@sahil9001
Copy link
Author

Screenshot 2024-10-24 at 12 14 02 AM
It is there @AlfredoG87 , is this correct?

@Nana-EC Nana-EC requested review from georgi-l95 and quiet-node and removed request for stoyanov-st October 23, 2024 19:15
AlfredoG87
AlfredoG87 previously approved these changes Oct 23, 2024
Copy link
Collaborator

@AlfredoG87 AlfredoG87 left a comment

Choose a reason for hiding this comment

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

LGTM!
TYVM for your contribution 💯

@AlfredoG87
Copy link
Collaborator

Screenshot 2024-10-24 at 12 14 02 AM It is there @AlfredoG87 , is this correct?

No, as you can see you can still see the red warning sign. if the blank line has blank spaces (spaces) is no longer blank

Signed-off-by: Sahil Silare <sahilsilare@gmail.com>
Signed-off-by: Sahil Silare <sahilsilare@gmail.com>
@sahil9001
Copy link
Author

Screenshot 2024-10-24 at 12 14 02 AM It is there @AlfredoG87 , is this correct?

No, as you can see you can still see the red warning sign. if the blank line has blank spaces (spaces) is no longer blank

Fixed, is this eligible for hacktoberfest swags? @AlfredoG87

Copy link

sonarcloud bot commented Oct 24, 2024

Copy link
Collaborator

@georgi-l95 georgi-l95 left a comment

Choose a reason for hiding this comment

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

LG, tyvm for your contribution @sahil9001

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers hacktoberfest Issues shown by lists for the Hacktoberfest and made for newcomers to do the first contribution. Internal For changes that affect the project's internal workings but not its outward-facing functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a redis instance to relay docker-compose
5 participants