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

[Issue 11473] [Python] Fix fields that are ignoring the required key argument #11508

Merged

Conversation

HugoPelletier
Copy link
Contributor

Fixes #11473

Motivation

By default, classes that inherit from Fields have a required key argument which is False.

If an attribute is not declared with a value, an exception is raised.

Invalid type '<class' NoneType '>' for field 'name'. Expected a string

Modifications

The modifications made affect 2 classes: Field and String. In both cases, the validate_type method has been modified to take into account arguments inherited from the Field class, including required and default.

Validation looks to see if the field is required. If it is not required (required=False) and the value does not exist, the default value is returned.

This modification removes the error and allows to integrate the different types of fields based on the definition of the key arguments of the Field class

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added unit teststo validate that all types of fields are returning the expected default value and are not raising an error anymore

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: yes
  • The default values of configurations: no
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: no

Documentation

For commiter

No documentation update is required.
The implementation of any fields is not include in the documentation.

Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

LGTM

@HugoPelletier
Copy link
Contributor Author

Is there a way to replay the CI pipeline?
Looks like one of the checks failed, but not related to my PR.

@BewareMyPower
Copy link
Contributor

/pulsarbot run-failure-checks

@sijie sijie added this to the 2.9.0 milestone Aug 4, 2021
@sijie sijie merged commit b80b0be into apache:master Aug 4, 2021
@HugoPelletier HugoPelletier deleted the fix_python_required_false_fields branch August 4, 2021 11:35
codelipenghui pushed a commit that referenced this pull request Aug 4, 2021
… argument (#11508)

Fixes #11473 


### Motivation

By default, classes that inherit from Fields have a `required` key argument which is False.

If an attribute is not declared with a value, an exception is raised.

```python
Invalid type '<class' NoneType '>' for field 'name'. Expected a string
```

### Modifications

The modifications made affect 2 classes: Field and String. In both cases, the `validate_type` method has been modified to take into account arguments inherited from the Field class, including` required` and `default`.

Validation looks to see if the field is required. If it is not required (`required=False`) and the value does not exist, the default value is returned.

This modification removes the error and allows to integrate the different types of fields based on the definition of the key arguments of the Field class

(cherry picked from commit b80b0be)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Aug 4, 2021
codelipenghui pushed a commit that referenced this pull request Aug 5, 2021
… argument (#11508)

Fixes #11473 


### Motivation

By default, classes that inherit from Fields have a `required` key argument which is False.

If an attribute is not declared with a value, an exception is raised.

```python
Invalid type '<class' NoneType '>' for field 'name'. Expected a string
```

### Modifications

The modifications made affect 2 classes: Field and String. In both cases, the `validate_type` method has been modified to take into account arguments inherited from the Field class, including` required` and `default`.

Validation looks to see if the field is required. If it is not required (`required=False`) and the value does not exist, the default value is returned.

This modification removes the error and allows to integrate the different types of fields based on the definition of the key arguments of the Field class

(cherry picked from commit b80b0be)
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Aug 5, 2021
LeBW pushed a commit to LeBW/pulsar that referenced this pull request Aug 9, 2021
… argument (apache#11508)

Fixes apache#11473 


### Motivation

By default, classes that inherit from Fields have a `required` key argument which is False.

If an attribute is not declared with a value, an exception is raised.

```python
Invalid type '<class' NoneType '>' for field 'name'. Expected a string
```

### Modifications

The modifications made affect 2 classes: Field and String. In both cases, the `validate_type` method has been modified to take into account arguments inherited from the Field class, including` required` and `default`.

Validation looks to see if the field is required. If it is not required (`required=False`) and the value does not exist, the default value is returned.

This modification removes the error and allows to integrate the different types of fields based on the definition of the key arguments of the Field class
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
… argument (apache#11508)

Fixes apache#11473 


### Motivation

By default, classes that inherit from Fields have a `required` key argument which is False.

If an attribute is not declared with a value, an exception is raised.

```python
Invalid type '<class' NoneType '>' for field 'name'. Expected a string
```

### Modifications

The modifications made affect 2 classes: Field and String. In both cases, the `validate_type` method has been modified to take into account arguments inherited from the Field class, including` required` and `default`.

Validation looks to see if the field is required. If it is not required (`required=False`) and the value does not exist, the default value is returned.

This modification removes the error and allows to integrate the different types of fields based on the definition of the key arguments of the Field class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.7 Archived: 2.7 is end of life cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.7.4 release/2.8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Python Client] Encode a struct schema object with None field
6 participants