Skip to content

Simplify parseResponse() method #41

Description

@gawbul

Priority: Medium

Related TODO: ✅ Simplify parseResponse

Issue

The parseResponse() method mixes multiple responsibilities:

  • Response parsing
  • Rate limit extraction
  • Retry decision logic
  • Content type handling

This violates Single Responsibility Principle and makes the method harder to test and maintain.

Current Code

# pyensemblrest/ensemblrest.py:320-352
def parseResponse(self, resp: Response | FakeResponse, content_type: str = "application/json") -> Any:
    logger.debug("Got %s" % resp.text)
    self.last_response = resp
    
    # Rate limit extraction
    (self.rate_reset, self.rate_limit, ...) = self.__get_rate_limit(resp.headers)
    
    # Retry decision
    if self.__check_retry(resp):
        return self.__retry_request()
    
    # Content parsing
    if content_type == "application/json":
        content = json.loads(resp.text)
    else:
        content = resp.text
    
    return content

Recommendation

Extract into focused methods:

def parseResponse(self, resp: Response | FakeResponse, content_type: str = "application/json") -> Any:
    """Parse and validate API response."""
    logger.debug("Got %s" % resp.text)
    self.last_response = resp
    self._extract_rate_limits(resp.headers)
    
    if self._should_retry(resp):
        return self.__retry_request()
    
    return self._parse_content(resp.text, content_type)

def _extract_rate_limits(self, headers: CaseInsensitiveDict[str] | dict[str, Any]) -> None:
    """Extract rate limit information from response headers."""
    (self.rate_reset, self.rate_limit, self.rate_remaining, 
     self.retry_after, self.rate_period) = self.__get_rate_limit(headers)

def _should_retry(self, resp: Response | FakeResponse) -> bool:
    """Determine if the request should be retried."""
    return self.__check_retry(resp)

def _parse_content(self, text: str, content_type: str) -> Any:
    """Parse response content based on content type."""
    if content_type == "application/json":
        return json.loads(text)
    return text

Benefits

  • Easier to test each component
  • Better separation of concerns
  • More maintainable code
  • Clearer intent

Files to Update

  • pyensemblrest/ensemblrest.py

Testing

  • Run full test suite
  • Add unit tests for individual parsing methods

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions