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

Cleanup and test of garden connection configs #5

Draft
wants to merge 1 commit into
base: garden_connection_config_feature
Choose a base branch
from

Conversation

jlrpnbbngtn
Copy link
Owner

@jlrpnbbngtn jlrpnbbngtn commented Jan 25, 2022

Testing

This PR accomplishes two main goals:

  1. to_brewtils now correctly processes systems associated with a garden via an update to make_object of GardenSchema in garden_schema.py. The unit test in test/db/schema/garden_schema_test.py demonstrates this.

  2. The functions that manipulate gardens in garden.py now call a function clean_garden_connection_params at points where the garden is inserted or retrieved from the database. This function only cleans up the garden connection parameters and passes the rest of the garden through untouched.

    This addresses the problem where a user upgrades to code that includes the garden schema but the database in the existing system may have gardens where the connection parameters are squirrely (and would therefore fail validation).

    There are comprehensive units tests to cover all of the possible permutations of connection parameters, explained further below.

The unit tests for the connection parameters fall along three main lines: the garden.connection_type is one of LOCAL, HTTP or STOMP. The tests thus verify each of the following cases is handled correctly:

  1. LOCAL
    a. no connection params
    b. connection params that are not empty
  2. HTTP
    a. good http, empty stomp
    b. good http, no stomp
    c. good http, bad stomp
    d. bad http, no stomp
    e. bad http, good stomp
    f. bad http, bad stomp
    g. no http, no stomp
    h. no http, good stomp
    i. no http, bad stomp
  3. STOMP
    a. empty http, good stomp
    b. no http, good stomp
    c. bad http, good stomp
    d. no http, no stomp
    e. no http, bad stomp
    f. good http, bad stomp
    g. bad http, bad stomp

The unit tests are tedious and repetitive (it would have taken too long to refactor them to make them simple and elegant for no real benefit), but they should cover all the combination of connection types and connection parameters.

return {config_map[key]: defaults[key] for key in config_map}


def _validate_http_connection_params(conn_params):
Copy link
Owner Author

Choose a reason for hiding this comment

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

Here starts the code to cleanup bad connection configuration in Gardens (many of the changes in this file are related to renaming brewtils objects).

@jlrpnbbngtn jlrpnbbngtn marked this pull request as ready for review January 25, 2022 18:44
@jlrpnbbngtn jlrpnbbngtn marked this pull request as draft February 2, 2022 21:42
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.

1 participant