Skip to content

Fix TODOs and improve code quality#261

Open
vicky-dx wants to merge 1 commit into
kobotoolbox:masterfrom
vicky-dx:fix/resolve-todos-and-improve-code
Open

Fix TODOs and improve code quality#261
vicky-dx wants to merge 1 commit into
kobotoolbox:masterfrom
vicky-dx:fix/resolve-todos-and-improve-code

Conversation

@vicky-dx

Copy link
Copy Markdown
  • Improve stdout/stderr handling in CLI.run_command()

    • Separate stdout and stderr capture
    • Better error reporting and debugging
    • Fix subprocess.run() usage
  • Remove code duplication in database user management

    • Create reusable __handle_db_user_changes() helper
    • Eliminate ~70 lines of duplicated code
    • Apply DRY principle
  • Enhance directory validation

    • Add robust __is_valid_kobo_install_directory() method
    • Multi-file validation (git repo + key files)
    • More reliable installation detection
  • Add customizable proxy port feature

    • Allow users to customize exposed_nginx_docker_port
    • Advanced mode option for external HTTP/HTTPS ports
    • Maintain backward compatibility
  • Fix bugs

    • Resolve undefined backend_role variable
    • Fix type hint compilation errors
  • Update .gitignore

    • Add Python best practices
    • Ignore venv, cache, IDE, and OS files
  • Update tests

    • Fix test mocks for new CLI.run_command() signature
    • Support both docker-compose and docker compose formats
    • All 34 config tests passing

Fixes existing TODO comments and improves code maintainability.

- Improve stdout/stderr handling in CLI.run_command()
  * Separate stdout and stderr capture
  * Better error reporting and debugging
  * Fix subprocess.run() usage

- Remove code duplication in database user management
  * Create reusable __handle_db_user_changes() helper
  * Eliminate ~70 lines of duplicated code
  * Apply DRY principle

- Enhance directory validation
  * Add robust __is_valid_kobo_install_directory() method
  * Multi-file validation (git repo + key files)
  * More reliable installation detection

- Add customizable proxy port feature
  * Allow users to customize exposed_nginx_docker_port
  * Advanced mode option for external HTTP/HTTPS ports
  * Maintain backward compatibility

- Fix bugs
  * Resolve undefined backend_role variable
  * Fix type hint compilation errors

- Update .gitignore
  * Add Python best practices
  * Ignore venv, cache, IDE, and OS files

- Update tests
  * Fix test mocks for new CLI.run_command() signature
  * Support both docker-compose and docker compose formats
  * All 34 config tests passing

Fixes existing TODO comments and improves code maintainability.
@Akuukis Akuukis requested a review from noliveleger October 27, 2025 09:14
@Akuukis Akuukis assigned Akuukis and noliveleger and unassigned Akuukis Oct 27, 2025
Comment thread helpers/config.py
Comment on lines +1328 to +1331
elif not self.__dict.get('mongo_secured'):
# Only password changed or first time securing
self.__write_upsert_db_users_trigger_file('', 'mongo')
self.__dict['mongo_secured'] = True

@noliveleger noliveleger Nov 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It never enters this block on password (only) change. So the file which triggers the password change in MongoDB is never created.

Pull main and merge it in your branch, you will see that test_update_mongo_passwords is now failing!

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