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

Allow running without Redis, without printing errors #5384

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

buger
Copy link
Member

@buger buger commented Aug 3, 2023

Tyk already supports running without Redis, because it support Redis failure handling, and reconnection mechanism.

What this PR adds is allow set TYK_GW_STORAGE_TYPE to empty value (actually anything non "redis"), disable auto-reconnects, and turns on fallback mode. It also makes it not print error messages every 10 seconds.

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

Tyk already supports running without Redis, because it support Redis failure handling, and reconnection mechanism.

What this PR adds is allow set TYK_GW_STORAGE_TYPE to empty value (actually anything non "redis"), disable auto-reconnects, and turns on fallback mode. It also makes it not print error messages every 10 seconds.
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

PR Analysis

  • 🎯 Main theme: This PR allows Tyk to run without Redis, without printing errors. It introduces a fallback mode and disables auto-reconnects when Redis is not set as the storage type.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: No
  • Focused PR: Yes, the PR has a clear and coherent title and description, and all PR code diff changes are properly derived from the title and description.
  • 🔒 Security concerns: No, the changes made in this PR do not introduce any apparent security concerns. The changes primarily involve configuration and error handling for the storage type.

PR Feedback

  • 💡 General PR suggestions: The PR is generally well-structured and the changes are clear. However, it would be beneficial to include tests that verify the new behavior when Redis is not set as the storage type. Additionally, the PR description could be improved by providing more details on how the changes have been tested.

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve: Suggest improvements to the code in the PR.
/ask <QUESTION>: Pose a question about the PR.

To edit any configuration parameter from 'configuration.toml', add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 3, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

35.7% 35.7% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@Tyk-ITS
Copy link
Contributor

Tyk-ITS commented Aug 3, 2023

API tests result: success
Branch used: refs/pull/5384/merge
Commit:
Triggered by: pull_request (@buger)
Execution page

@jonathanviber
Copy link

I'm guessing this error should be easy to solve:
_ time="Aug 03 12:16:17" level=error msg="Could not load Go-plugin" error="plugin.Open("/opt/tyk-gateway/middleware/testplugin"): plugin was built with a different version of package github.com/xeipuuv/gojsonpointer" mwPath=/opt/tyk-gateway/middleware/testplugin.so mwSymbolName=AddFooBarHeader_

A few others look like they relate to REDIS checks that failed. Has this been abandoned or is it still in progress?

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