Skip to content

Conversation

@mousavian
Copy link
Contributor

@mousavian mousavian commented Apr 28, 2020

When using this SDK with ansible inventory script, it fails with expected string or bytes-like object error when one or few required fields are not provided in oci config, but initialised with None value in the inventory script.

Here's the stack trace when the user is missing:

$ ./inventory/oci/oci_inventory.py --list --debug
Executing in debug mode.
Traceback (most recent call last):
  File "./inventory/oci/oci_inventory.py", line 374, in __init__
    self.identity_client = self.create_service_client(IdentityClient)
  File "./inventory/oci/oci_inventory.py", line 464, in create_service_client
    client = service_client_class(params, **kwargs)
  File "./venv/lib/python3.7/site-packages/oci/identity/identity_client.py", line 60, in __init__
    validate_config(config, signer=kwargs.get('signer'))
  File "./venv/lib/python3.7/site-packages/oci/config.py", line 112, in validate_config
    if not pattern.match(config[key]):
TypeError: expected string or bytes-like object

expected string or bytes-like object

@bhagwatvyas
Copy link
Member

Hi @mousavian, sorry for delayed response. We recently made some changes to the config logic, could you please rebase and update this PR?

@mousavian
Copy link
Contributor Author

@bhagwatvyas done

for required_key in REQUIRED:
fallback_key = REQUIRED_FALLBACKS.get(required_key)
if required_key not in config and fallback_key not in config:
if (required_key not in config or config[required_key] is None) and fallback_key not in config:
Copy link
Member

Choose a reason for hiding this comment

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

if we're doing this to handle config[required_key] = None I think it makes sense to make the same fix for config[fallback_key] = None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. Thanks for pointing it out @mross22.
I've updated the branch with changes.

@nalsaber
Copy link
Member

It looks fine from Ansible perspective

Signed-off-by: Rahman Mousavian <rahman.mousavian@oracle.com>
@mousavian
Copy link
Contributor Author

I've applied requested changes. Let me know if anything else is needed.

@bhagwatvyas bhagwatvyas self-requested a review August 21, 2020 22:13
@bhagwatvyas bhagwatvyas merged commit 835b38c into oracle:master Aug 21, 2020
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.

4 participants