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/ Redis HA fix #283

Merged
merged 10 commits into from
Sep 10, 2024
Merged

Feat/ Redis HA fix #283

merged 10 commits into from
Sep 10, 2024

Conversation

alithethird
Copy link
Contributor

@alithethird alithethird commented Sep 3, 2024

Includes the Redis HA fix. This fixes 2 different bugs:
1- https://bugs.launchpad.net/juju/+bug/2063087
2- #268

Overview

Started using app databag for Redis hostname. Add tests to test out Redis in HA mode.

Rationale

Previously was not able to use Redis in HA mode

Juju Events Changes

Module Changes

Library Changes

Checklist

@alithethird alithethird self-assigned this Sep 3, 2024
@alithethird alithethird changed the title Feat/ redis ha fix Feat/ Redis HA fix Sep 4, 2024
@alithethird alithethird marked this pull request as ready for review September 4, 2024 12:03
@alithethird alithethird requested a review from a team as a code owner September 4, 2024 12:03
Copy link
Contributor

@javierdelapuente javierdelapuente left a comment

Choose a reason for hiding this comment

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

One question, what are now the restrictions to integrate with Redis? A specific version?

Should we fallback to the previous functionality if "leader-host" is not there? Otherwise, should we tell the user that this charm requires a Redis charm revision greater than X?

src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
@alithethird
Copy link
Contributor Author

One question, what are now the restrictions to integrate with Redis? A specific version?

Should we fallback to the previous functionality if "leader-host" is not there? Otherwise, should we tell the user that this charm requires a Redis charm revision greater than X?

Ok, that is a great question. I did not think of that. I think I should ask @gregory-schiano about this.

I think the fallback to unit data if leader-host doesn't exist is a great idea. What do you think?

src/charm.py Outdated Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
@alithethird alithethird added enhancement New feature or request good first issue Good for newcomers trivial labels Sep 5, 2024
Copy link
Contributor

github-actions bot commented Sep 9, 2024

Test coverage for 8aa08d6

Name              Stmts   Miss Branch BrPart  Cover   Missing
-------------------------------------------------------------
src/charm.py        373     41     94     15    88%   184, 192-193, 205, 337->345, 378->383, 395, 583-585, 590-591, 603-605, 610-611, 623-625, 648-650, 692->exit, 702->exit, 751-754, 764-765, 789-790, 802-803, 830-832, 834->839, 836-837, 879-880, 896-897, 907->exit, 921
src/database.py      29      1      8      1    95%   56
-------------------------------------------------------------
TOTAL               402     42    102     16    88%

Static code analysis report

Run started:2024-09-09 06:25:23.083683

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 2359
  Total lines skipped (#nosec): 2
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

Copy link
Contributor

@amandahla amandahla left a comment

Choose a reason for hiding this comment

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

LGTM

@alithethird alithethird merged commit 8175cc2 into main Sep 10, 2024
19 checks passed
@alithethird alithethird deleted the feat/-Redis-HA-fix branch September 10, 2024 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers Libraries: OK trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants