Skip to content

Conversation

rocky-d
Copy link

@rocky-d rocky-d commented Oct 3, 2025

Motivation and Context

The ResourceServerSettings class had a custom __init__ method that simply called super().__init__(**data) without adding any additional functionality. This is redundant since Pydantic's BaseSettings already provides the same behavior by default.

The TODO comment from Marcelo indicated uncertainty about whether this constructor was needed. After reviewing the code, it's clear that:

  • No custom initialization logic is required
  • The parent class BaseSettings already handles environment variable loading and validation
  • The custom __init__ method provides no additional value

How Has This Been Tested?

  • Verified that ResourceServerSettings still properly loads configuration from environment variables
  • Confirmed that the MCP Resource Server starts correctly without the custom constructor
  • Tested OAuth authentication flow remains functional
  • Validated that all existing functionality works as expected

Breaking Changes

None. This is a purely internal change that removes redundant code without affecting the public API or behavior of the class.

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)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

This change aligns with the principle of "don't repeat yourself" (DRY) and removes code that was flagged as potentially unnecessary by the original author. The BaseSettings class from Pydantic already provides all the initialization functionality needed for configuration management.

@felixweinberger felixweinberger requested a review from Kludex October 3, 2025 12:31
@felixweinberger felixweinberger added the needs more eyes Needs alignment among maintainers whether this is something we want to add label Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more eyes Needs alignment among maintainers whether this is something we want to add
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants