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

fix: More renaming of "core" to "external" redis #340

Merged
merged 16 commits into from
Feb 10, 2025
Merged

Conversation

j7m4
Copy link
Contributor

@j7m4 j7m4 commented Feb 10, 2025

Summary by CodeRabbit

  • New Features
    • Introduced an optional caching layer that allows users to control whether a caching resource is deployed.
    • Expanded Redis connectivity options, enabling a choice between using internally managed or externally provided Redis deployments.
    • Added new configuration settings, enhancing deployment flexibility for your infrastructure without impacting existing functionality.

Copy link

linear bot commented Feb 10, 2025

Copy link

coderabbitai bot commented Feb 10, 2025

Walkthrough

The changes update several Terraform configurations to support conditional creation of resources and external Redis configuration. A new variable, create_elasticache, is added to control Elasticache resource creation in the public DNS example. The main configuration now conditionally selects between external and internal Redis settings based on the use_external_redis and create_elasticache variables. Additionally, three new external Redis variables are defined, and a minor update in the Aurora module simplifies the namespace reference in security group tags.

Changes

File(s) Change Summary
examples/…/main.tf, examples/…/variables.tf Added the create_elasticache variable to the wandb_infra module and declared it (bool, default true) to conditionally create an Elasticache Redis resource.
main.tf Updated the Redis configuration to conditionally use external Redis settings (external_redis_host and external_redis_port) when use_external_redis is true; otherwise falls back to internal settings with additional TLS/TTL adjustments if create_elasticache is true.
variables.tf Introduced new variables for external Redis configuration: use_external_redis (bool, default false), external_redis_host (string, default null), and external_redis_port (string, default null).
modules/…/main.tf Modified the Aurora module by replacing string interpolation of namespace in security_group_tags with a direct variable reference.

Sequence Diagram(s)

sequenceDiagram
    participant TF as Terraform
    participant RedisConfig as Redis Configurator

    TF->>RedisConfig: Evaluate use_external_redis?
    alt use_external_redis is true
        RedisConfig-->>TF: Return external_redis_host and external_redis_port
    else
        RedisConfig-->>TF: Return internal Redis host and port
        alt create_elasticache is true
            TF->>RedisConfig: Append TLS and TTL settings
        end
    end
Loading

Possibly related PRs

Suggested reviewers

  • zacharyblasczyk
  • theishshah

Poem

I'm a rabbit with a coding streak,
Digging in Terraform, sleek and unique.
Elastic hops with each variable in sight,
External Redis makes my day just right.
I nibble on changes as lines unfold,
Celebrating commits both fresh and bold!
Hop on, dear devs, in infrastructure gold! 🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20d2c3d and dbc89e5.

📒 Files selected for processing (5)
  • examples/public-dns-external/main.tf (1 hunks)
  • examples/public-dns-external/variables.tf (1 hunks)
  • main.tf (1 hunks)
  • modules/database/main.tf (1 hunks)
  • variables.tf (1 hunks)
🔇 Additional comments (10)
examples/public-dns-external/main.tf (1)

53-54: Add create_elasticache Parameter to Module
The addition of the line
    create_elasticache = var.create_elasticache
in the wandb_infra module configuration is correctly implemented. This change enables conditional provisioning of an Elasticache Redis instance. Ensure that the module’s internal logic is updated to support this new parameter.

examples/public-dns-external/variables.tf (4)

135-139: Declare create_elasticache Variable
The new variable declaration for create_elasticache is well defined with type bool, a default value of true, and an appropriate description. This variable will control whether an Elasticache Redis instance is created.


523-527: Declare use_external_redis Variable
The use_external_redis variable is clearly defined to allow toggling between using an external Redis instance and the internally provisioned one. With a default of false, it ensures that external Redis will only be used when explicitly enabled.


529-533: Declare external_redis_host Variable
The external_redis_host variable is properly declared to capture the hostname for an externally created Redis instance. The default value of null indicates that it remains inactive unless set, which is the desired behavior.


535-539: Declare external_redis_port Variable
The declaration for external_redis_port is correct, with type string and a default of null. This variable will be used for defining the port when using an external Redis instance. Consider adding documentation or validation if specific format requirements are expected.

modules/database/main.tf (1)

136-136: Simplify Security Group Tag Reference
Replacing the previous string interpolation with a direct variable reference in
    security_group_tags = { "Namespace" : var.namespace }
improves clarity and reduces complexity. This adjustment is a good cleanup that does not affect functionality.

main.tf (1)

306-308: Enhance Redis Configuration with Conditional Logic
The updated Redis configuration in the wandb module now leverages the new use_external_redis flag to determine the correct host and port to use. When using an external Redis instance, the values from var.external_redis_host and var.external_redis_port are applied; otherwise, the configuration falls back to using internal module outputs (with additional TLS and TTL parameters for the port if create_elasticache is true). Please verify that downstream consumers correctly parse the port string with appended parameters.

variables.tf (3)

523-527: Introduce use_external_redis Variable
The new use_external_redis variable is clearly defined to offer a switch between internal and external Redis configurations. Its default value of false is appropriate to maintain existing behavior unless an external instance is explicitly provided.


529-533: Define external_redis_host Variable
This variable enables users to specify the host for an externally provisioned Redis instance. The default value of null ensures that the external configuration is only active when it is intentionally set.


535-539: Define external_redis_port Variable
The external_redis_port variable is correctly added to capture the port number for an external Redis instance. As a best practice, consider documenting acceptable values or adding a validation block to ensure consistency in its usage throughout the configuration.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@j7m4 j7m4 requested a review from theishshah February 10, 2025 19:15
@j7m4 j7m4 merged commit 7bdfc1b into main Feb 10, 2025
7 checks passed
@j7m4 j7m4 deleted the j7m4/infra-383-via-core branch February 10, 2025 20:52
jsbroks pushed a commit that referenced this pull request Feb 10, 2025
### [7.9.1](v7.9.0...v7.9.1) (2025-02-10)

### Bug Fixes

* More renaming of "core" to "external" redis ([#340](#340)) ([7bdfc1b](7bdfc1b))
@jsbroks
Copy link
Member

jsbroks commented Feb 10, 2025

This PR is included in version 7.9.1 🎉

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.

3 participants