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

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

sahil9001
Copy link
Contributor

@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
@sahil9001 sahil9001 requested a review from stoyanov-st 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.

@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 from 4424b1f to a47f78a Compare October 19, 2024 10:49
@sahil9001 sahil9001 requested a review from AlfredoG87 October 19, 2024 10:50
@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
Contributor 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.

@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

@sahil9001 sahil9001 requested a review from AlfredoG87 October 23, 2024 18:15
@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 sahil9001 force-pushed the add-redis-in-docker-compose branch from bada091 to 039d029 Compare October 23, 2024 18:26
@sahil9001
Copy link
Contributor 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
Contributor 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
Contributor 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

Copy link
Member

@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

@ebadiere ebadiere modified the milestones: 0.59.0, 0.60.0 Oct 30, 2024
@quiet-node
Copy link
Member

Hello @sahil9001, we currently just pushed in an internal fix for the PR Check / Title Check. Could you please pull the latest changes from remote hashgraph:main to update your this forked branch?

@quiet-node quiet-node added the good first issue candidate Issues that can become a good first issue but need more description/context. label Nov 6, 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

@Nana-EC Nana-EC merged commit d635301 into hashgraph:main Nov 12, 2024
36 of 37 checks passed
@sahil9001
Copy link
Contributor Author

Hey @AlfredoG87, is this PR eligible for hacktoberfest swags?

@AlfredoG87
Copy link
Collaborator

@sahil9001 am not sure, but I would not see why not.

@sahil9001
Copy link
Contributor Author

@AlfredoG87 I have not received any communication regarding the swags, can you check please?

@AlfredoG87
Copy link
Collaborator

I am sorry, that is not my department, probably the persons in charge are on PTO due to the holidays.
I will ask around next week that everyone is back and get back to you.

@sahil9001
Copy link
Contributor Author

Thanks for the response!

@sahil9001
Copy link
Contributor Author

Hey @AlfredoG87 , do we have any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue candidate Issues that can become a good first issue but need more description/context. 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
6 participants