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

fix:_check_proto_obj_attr_exist does not handle None properly #166

Conversation

gyar-denim
Copy link
Contributor

None check in _check_proto_obj_attr_exist is not handled properly.

# is buggy for None check.
if not obj:  

# correct way
if obj is None:

# Below code makes one example why it is buggy. 
from dfcx_scrapi.builders.fulfillments import FulfillmentBuilder
from google.cloud.dialogflowcx_v3beta1.types import Fulfillment


def main():
    fulfilment_builder = FulfillmentBuilder()
    fullfilment_obj = fulfilment_builder.create_new_proto_obj()
    assert fulfilment_builder.proto_obj is not None
    assert isinstance(fulfilment_builder.proto_obj, Fulfillment)
    assert isinstance(fullfilment_obj, Fulfillment)

    assert (not fulfilment_builder.proto_obj) is True
    # fails because there is a bug in _check_proto_obj_attr_exist()
    fulfilment_builder._check_proto_obj_attr_exist()

    """
        def _check_proto_obj_attr_exist(self):
        
        # BUG HERE : should be if self.proto_obj is None
        
        if not self.proto_obj:
            raise ValueError(
                "There is no proto_obj!"
                "\nUse `create_new_proto_obj` or `load_proto_obj` to continue."
            )
        elif not isinstance(self.proto_obj, self._proto_type):  # pylint: disable=W1116
            raise ValueError(
                f"proto_obj is not {self._proto_type_str} type."
                "\nPlease create or load the correct type to continue."
            )
    """


if __name__ == "__main__":
    main()

Copy link

google-cla bot commented Jan 10, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@MRyderOC
Copy link
Collaborator

Thanks for pointing this out. This was a known issue but unfortunately I forgot to solve the bug. Please sign the CLA using the email you've created your GitHub account with so we can merge the PR.

@gyar-denim
Copy link
Contributor Author

gyar-denim commented Jan 10, 2024 via email

@MRyderOC
Copy link
Collaborator

thanks for signing that!
@kmaphoenix LGTM!

@kmaphoenix kmaphoenix merged commit db724d6 into GoogleCloudPlatform:main Jan 10, 2024
2 checks passed
@MRyderOC MRyderOC mentioned this pull request Feb 22, 2024
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