Skip to content

tamar_k_parametergroup#346

Open
saraNoigershel wants to merge 6 commits intomainfrom
new_kt_db
Open

tamar_k_parametergroup#346
saraNoigershel wants to merge 6 commits intomainfrom
new_kt_db

Conversation

@saraNoigershel
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Collaborator

@riki7649255 riki7649255 left a comment

Choose a reason for hiding this comment

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

DB/NEW_KT_DB/DataAccess/DBClusterParameterGroupManager.py:

  1. rename table_schema to table_structure
  2. line 37- remove empty lines

DB/NEW_KT_DB/Service/Classes/DBClusterParameterGroupService.py
3. rename function camel_to_snake_case() to convert_camel_case_string_to_snake()
4. rename function convert_dict_keys_to_snake_case() to convert_dict_keys_from_camel_case_to_snake()

DB/NEW_KT_DB/Test/DBClusterParameterGroupTests.py
5. remove commented code and other dev leftobers if exist
6. I see you used multiple times the variables group_name, file_name, and few more. it is better to put the usefull static variables inside init() method of your test class, and the dynamic variables inside internal function that will change the value assigned in the init() method too.

DB/NEW_KT_DB/Test/GeneralTests.py
7. this file should contain general tests only, or internal functions. so the function def delete_file_if_exists(storage_manager, file_name): you created suppose to be in the storage.object_manager.

Copy link
Copy Markdown
Collaborator

@riki7649255 riki7649255 left a comment

Choose a reason for hiding this comment

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

DB/NEW_KT_DB/Test/DBClusterParameterGroupTests.py

  1. I see you used multiple times the variables group_name, file_name, and few more. it is better to put the usefull static variables inside init() method of your test class, and the dynamic variables inside internal function that will change the value assigned in the init() method too.

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