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 'NoneType' object has no attribute 'get' #1191

Closed
wants to merge 1 commit into from

Conversation

freyes
Copy link
Member

@freyes freyes commented Mar 12, 2024

There are situations where the action object returned by libjuju may not have the 'results' attribute which leads to using the default value when running ".get('results')" which it's None.

This change sets the default value to an empty dictionary to allow the following ".get('Stdout', '')" succeed.

There are situations where the action object returned by libjuju may not
have the 'results' attribute which leads to using the default value when
running ".get('results')" which it's None.

This change sets the default value to an empty dictionary to allow the
following ".get('Stdout', '')" succeed.
@freyes
Copy link
Member Author

freyes commented Mar 12, 2024

This PR is being exercised by this gerrit change - https://review.opendev.org/c/openstack/charm-mysql-innodb-cluster/+/912487

@freyes freyes requested review from wolsen and sabaini March 12, 2024 20:46
Copy link
Contributor

@sabaini sabaini left a comment

Choose a reason for hiding this comment

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

+1 as a tactical fix

The results attribute for unit.run() missing sometime smells a bit like a bug in libjuju though?

cheers!

@@ -238,7 +238,7 @@ async def _check_ca_present(model, ca_files):
for unit in units:
try:
output = await unit.run('cat {}'.format(ca_file))
contents = output.data.get('results').get('Stdout', '')
contents = output.data.get('results', {}).get('Stdout', '')
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I think the problem here is that this is invoking direct juju invocations rather than the zaza model. The zaza model has logic to deal with the async result differences between libjuju 2.x and 3.x, but this code doesn't have that.

@wolsen
Copy link
Contributor

wolsen commented Mar 19, 2024

I'm closing this pull request as we used PR #1192 instead, which uses the underlying zaza.model changes to not use the unit.run directly.

@wolsen wolsen closed this Mar 19, 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