Skip to content

Conversation

ArnyminerZ
Copy link
Member

@ArnyminerZ ArnyminerZ commented Sep 4, 2025

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 <[email protected]>
@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
Copilot

This comment was marked as outdated.

Copy link
Member

@sunkup sunkup left a comment

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
Member Author

Where is the sense in leaving followRedirects = false ?

Ooops absolutely my fault

@ArnyminerZ ArnyminerZ requested a review from sunkup September 4, 2025 10:15
Copy link
Member

@sunkup sunkup left a comment

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

@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
Copy link
Contributor

@Copilot Copilot AI left a comment

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
2 participants