Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CIVIS-9192] ENH clean up database credential IDs at civis.APIClient #502

Merged
merged 5 commits into from
Nov 11, 2024

Conversation

jacksonlee-civis
Copy link
Member

This pull request tidies up the methods and properties of a civis.APIClient instance related to database credential IDs:

  • Deprecate the method get_database_credential_id, with no replacement planned. This method is no longer compatible with the current state of Civis Platform.
  • Add the new property default_database_credential_id, which is effectively an alias to the current less-clearly named property default_credential.
  • Deprecate the property default_credential in favor of the new default_database_credential_id.

  • (For Civis employees only) Reference to a relevant ticket in the pull request title
  • Changelog entry added to CHANGELOG.md at the repo's root level
  • Description of change in the pull request description
  • If applicable, unit tests have been added and/or updated
  • The CircleCI builds have all passed

Copy link

@leanne73 leanne73 left a comment

Choose a reason for hiding this comment

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

One question. Generally this looks great! Thanks for taking on this cleanup.

Copy link

@leanne73 leanne73 left a comment

Choose a reason for hiding this comment

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

LGTM!

@jacksonlee-civis jacksonlee-civis merged commit 74bb82f into main Nov 11, 2024
2 checks passed
@jacksonlee-civis jacksonlee-civis deleted the civis-9192-get-database-credential-id branch November 11, 2024 20:38
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.

2 participants