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

Refactor GKECreateClusterOperator's body validation #31923

Merged

Conversation

moiseenkov
Copy link
Contributor

@moiseenkov moiseenkov commented Jun 15, 2023

Refactored GKECreateClusterOperator's body validation because it is really hard to understand current implementation:

  1. Separated validation logic from the field value retrieval logic by moving out the latter into a separate method:
def _body_field(self, field_name: str, default_value: Any = None) -> Any:
    """Extracts the value of the given field name."""
    if isinstance(self.body, dict):
        return self.body.get(field_name, default_value)
    else:
        return getattr(self.body, field_name, default_value)
  1. Separated validation of the body fields from each other.
  2. Added warning messages when deprecated fields are used.
  3. Current behavior of the operator wasn't changed.

@boring-cyborg boring-cyborg bot added provider:cncf-kubernetes Kubernetes provider related issues area:providers provider:google Google (including GCP) related issues labels Jun 15, 2023
@moiseenkov moiseenkov force-pushed the gke_create_cluster_operator_validate_body branch 23 times, most recently from 713b22e to 5d461c2 Compare June 22, 2023 07:06
@moiseenkov moiseenkov force-pushed the gke_create_cluster_operator_validate_body branch 4 times, most recently from f41cfe9 to 5f84300 Compare June 23, 2023 09:35
@moiseenkov moiseenkov force-pushed the gke_create_cluster_operator_validate_body branch 20 times, most recently from bf99a8f to a36b9f5 Compare June 29, 2023 12:11
@moiseenkov moiseenkov force-pushed the gke_create_cluster_operator_validate_body branch from a36b9f5 to a80bf62 Compare June 29, 2023 14:03
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

LGTM. Pending tests passing. Nice small refactor.

@potiuk potiuk merged commit f3f69bf into apache:main Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:cncf-kubernetes Kubernetes provider related issues provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants