Skip to content

Conversation

@eiri
Copy link
Contributor

@eiri eiri commented Oct 17, 2025

PR summary

  • Convert CloudantBaseService to inherit from BaseService and use it as a super for CloudantV1 to avoid monkey-patching.
  • Pass BaseService's http client to CouchDbSessionAuthenticator, so it'd be used in token manager.
  • Rename class TestPageIterator in pagination's tests to BaseTestPageIterator to avoid test runner's warnings.

Fixes: s1030

Note: An existing issue is required before opening a PR.

PR Checklist

Please make sure that your PR fulfills the following requirements:

  • The commit message follows the
    Angular Commit Message Guidelines.
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • New tests
  • Build/CI related changes
  • Documentation content changes
  • Other (please describe)

What is the current behavior?

We monkey-patch methods of CloudantV1 to get extended functionality for generated sdk

What is the new behavior?

Extend functionality in base class and switch CloudantV1 to inherit from it

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Copy link
Member

@ricellis ricellis left a comment

Choose a reason for hiding this comment

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

This looks good to me, but it does need some copyright date updates

@eiri
Copy link
Contributor Author

eiri commented Oct 17, 2025

@ricellis good catch, but I keep it draft, because remembered that we also want to reuse client in session auth

@eiri eiri force-pushed the 1030-refactor-cloudant-base-service-class branch from c0f2cac to af755bc Compare October 17, 2025 15:04
@eiri eiri force-pushed the 1030-refactor-cloudant-base-service-class branch 2 times, most recently from ad16a45 to 858f318 Compare October 20, 2025 00:31
@eiri eiri marked this pull request as ready for review October 20, 2025 01:02
@eiri eiri requested a review from a team as a code owner October 20, 2025 01:02
@ricellis ricellis self-requested a review October 20, 2025 11:03
Copy link
Member

@ricellis ricellis left a comment

Choose a reason for hiding this comment

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

1 nit I missed first time round, but +1


def set_jar(self, jar):
"""Sets the cookie jar for the authenticator.
def set_http_client(self, http_client: Session, jar: RequestsCookieJar) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

since it is internal can we use the leading _ naming? ofc we'd need to update the matching callers too.

Suggested change
def set_http_client(self, http_client: Session, jar: RequestsCookieJar) -> None:
def _set_http_client(self, http_client: Session, jar: RequestsCookieJar) -> None:

@eiri eiri force-pushed the 1030-refactor-cloudant-base-service-class branch from f7842c1 to 16867c7 Compare October 20, 2025 15:13
@eiri eiri merged commit c2e376d into main Oct 20, 2025
10 checks passed
@eiri eiri deleted the 1030-refactor-cloudant-base-service-class branch October 20, 2025 15:33
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