Skip to content

Globally disable follow redirects#688

Merged
ArnyminerZ merged 3 commits into
devfrom
686-follow-redirects-is-not-disabled
Sep 9, 2025
Merged

Globally disable follow redirects#688
ArnyminerZ merged 3 commits into
devfrom
686-follow-redirects-is-not-disabled

Conversation

@ArnyminerZ

@ArnyminerZ ArnyminerZ commented Sep 4, 2025

Copy link
Copy Markdown
Member

Purpose

We are trying to disable redirection following in the engine config. We don't even want to disable redirection, so remove that line.

We do have to disable redirection, it's handled by CalendarFetcher. Following the docs, this is the way to disable it so it can be handled by it.

Short description

  • Remove the redirection disable in engine config.
  • Move the redirection disabling
  • Add a comment to make sure we don't get confused again in the future.

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

Signed-off-by: Arnau Mora <arnyminerz@proton.me>
@ArnyminerZ ArnyminerZ linked an issue Sep 4, 2025 that may be closed by this pull request
@ArnyminerZ
ArnyminerZ requested a review from Copilot September 4, 2025 09:13
@ArnyminerZ
ArnyminerZ marked this pull request as ready for review September 4, 2025 09:13
@ArnyminerZ
ArnyminerZ requested a review from sunkup September 4, 2025 09:13
@ArnyminerZ ArnyminerZ self-assigned this Sep 4, 2025
@ArnyminerZ ArnyminerZ added the refactoring Quality improvement of existing functions label Sep 4, 2025

This comment was marked as outdated.

@sunkup sunkup left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where is the sense in leaving followRedirects = false ?

@ArnyminerZ

Copy link
Copy Markdown
Member Author

Where is the sense in leaving followRedirects = false ?

Ooops absolutely my fault

Signed-off-by: Arnau Mora <arnyminerz@proton.me>
@ArnyminerZ
ArnyminerZ requested a review from sunkup September 4, 2025 10:15

@sunkup sunkup left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests need to be adapted

Signed-off-by: Arnau Mora <arnyminerz@proton.me>
@ArnyminerZ ArnyminerZ changed the title Enable follow redirects Globally disable follow redirects Sep 9, 2025
@ArnyminerZ
ArnyminerZ requested a review from Copilot September 9, 2025 08:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR relocates the redirect disabling configuration from engine-specific settings to a global HTTP client configuration to ensure consistent behavior across all engines.

  • Moves redirect disabling from conditional engine-specific blocks to global client configuration
  • Adds explanatory comment about redirect handling being managed by CalendarFetcher
  • Simplifies the configuration logic by removing conditional redirect settings

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ArnyminerZ
ArnyminerZ requested a review from sunkup September 9, 2025 08:34
@ArnyminerZ
ArnyminerZ merged commit 20958df into dev Sep 9, 2025
9 checks passed
@ArnyminerZ
ArnyminerZ deleted the 686-follow-redirects-is-not-disabled branch September 9, 2025 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring Quality improvement of existing functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ktor] Follow redirects is not disabled

3 participants