Skip to content

feat: use redis cache #3185

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

Merged
merged 1 commit into from
Jun 4, 2025
Merged

feat: use redis cache #3185

merged 1 commit into from
Jun 4, 2025

Conversation

shaohuzhang1
Copy link
Contributor

feat: use redis cache

Copy link

f2c-ci-robot bot commented Jun 4, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link

f2c-ci-robot bot commented Jun 4, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

"DB": self.get("REDIS_DB"),
"PASSWORD": self.get("REDIS_PASSWORD"),
"CONNECTION_POOL_KWARGS": {"max_connections": int(self.get("REDIS_MAX_CONNECTIONS"))}
},
},
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code has several issues and areas for improvement:

  1. Magic Numbers: The current configuration uses magic numbers that could be improved. Instead of hardcoding these values, use variables or constants.

  2. Error Handling: There's no error handling if the database settings aren't correctly configured.

  3. Security Concerns:

    • Hardcoded passwords should never be stored this way in source control. Ideally, they should be stored securely using environment variables or encrypted secrets management.
  4. Code Duplication: The get_cache_setting method is repeated with minor differences (backend type). This can be refactored to reduce redundancy.

Here's an optimized version of the code:

class Config(dict):
    DEFAULT_SETTINGS = {
        'LANGUAGE_CODE': 'zh-CN',
        "DEBUG": False,
        'REDIS_HOST': '127.0.0.1',  # Redis host
        'REDIS_PORT': 6379,       # Redis port
        'REDIS_PASSWORD': 'your_secure_password_here',  # Replace with secure password storage
        'REDIS_DB': 0,
        'REDIS_MAX_CONNECTIONS': 100
    }

    def __init__(self):
        super().__init__(Config.DEFAULT_SETTINGS)

    def set_config(self, key, value):
        self[key] = value

    # Functionality to override default configuration
    def load_settings_from_env(self):
        env_vars = {'REDIS_PASSWORD': 'password'}
        for var_name, default_value in env_vars.items():
            if var_name in os.environ:
                setattr(self, var_name.upper(), os.getenv(var_name))

   def update_with_env_overrides(self):
        # Placeholder for loading other environment overrides
        pass

    # Getters for specific configuration keys
    def get_language_code(self):
        return self['LANGUAGE_CODE']

    def get_debug(self) -> bool:
        return self['DEBUG']

    def get_db_setting(self) -> dict:
        db_info = {
            'ENGINE': 'django.db.backends.postgresql',  # Example DB backend
            'NAME': 'mydatabase',
            'USER': 'myuser',
            'PASSWORD': 'mypassword'  # Use environment variable here
        }
        db_info.update({'HOST': self['DATABASES_DEFAULT']['HOST'], 'PORT': self['DATABASES_DEFAULT'].get('PORT')})
        return db_info

    @staticmethod
    def get_cache_backend_url(config_obj):
        return (
            f"redis://:{config_obj['REDIS_PASSWORD']}@"
            f"{config_obj.get('REDIS_HOST')}:{config_obj.get('REDIS_PORT')}"
        )

    def get_cache_setting(self) -> dict:
        cache_backend_url = self.get_cache_backend_url(self)
        return {
            'default': {
                'BACKEND': 'django_redis.cache.RedisCache',
                'LOCATION': cache_backend_url,
                'OPTIONS': {
                    'CLIENT_CLASS': 'django_redis.client.DefaultClient',
                    "DB": int(self.get('REDIS_DB')),
                    "CONNECTION_POOL_KWARGS": {"max_connections": int(self.get('REDIS_MAX_CONNECTIONS'))}
                },
            },
        }


# Usage example
config_instance = Config()
config_instance.load_settings_from_env()

db_settings = config_instance.get_db_setting()
print(json.dumps(db_settings, indent=4))

Key Changes:

  • Environment Variable Support: Added load_settings_from_env and update_with_env_overrides methods to allow dynamic updates based on environment variables.
  • Variable Names: Defined standard variable names like DEFAULT_SETTINGS instead of hardcoded properties.
  • Configuration Updates: Removed magic numbers and used parameters where appropriate.
  • Separation of Concerns: Encapsulated logic into different functions for better organization.

@shaohuzhang1 shaohuzhang1 merged commit d92fdb9 into v2 Jun 4, 2025
3 of 5 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@v2@feat_use_redis_cache branch June 4, 2025 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant