Skip to content

Investigate refactoring or removing get_clp_connection_params_and_type method #1687

@coderabbitai

Description

@coderabbitai

Context

Related PR: #1655
Related comment: #1655 (comment)

Issue

The Database.get_clp_connection_params_and_type method in components/clp-py-utils/clp_py_utils/clp_config.py manually constructs a dictionary with connection parameters, which:

  • Duplicates logic that could be derived from dump_to_primitive_dict()
  • Is missing the ssl_cert field
  • May be doing more than necessary for some callers who only need a subset of information

Proposed Investigation

  1. Review all call sites to understand what fields are actually needed by each caller
  2. Consider replacing this method with a simpler "filter credentials" method
  3. For callers that need the full dictionary, consider refactoring to use dump_to_primitive_dict() as a base and then add/override only the necessary fields (credentials, table_prefix, type)

Suggested Refactor for Full Dictionary Use Case

d = self.dump_to_primitive_dict()
d["host"] = host  # Override with potentially modified host
d["credentials"] = {user_type: self.credentials[user_type].model_dump()}
d["table_prefix"] = CLP_METADATA_TABLE_PREFIX
# NOTE: clp-core does not distinguish between mysql and mariadb
d["type"] = DatabaseEngine.MYSQL.value
return d

Requested by: @junhaoliao

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions