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 #110 - peadm::status plan fails with new bolt version #111

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

logicminds
Copy link
Contributor

  • previously when getting data after a task is run, we fetched
    the result data. However, a newer version of bolt changed
    the key name to value and broke this code. This fixes that
    by fetching the newer key name and failing back to the old
    key name if the value does not exist.

@logicminds logicminds requested a review from a team as a code owner August 4, 2020 16:48
@reidmv
Copy link
Contributor

reidmv commented Aug 4, 2020

Interesting that they changed this. Do you know what version the break happened at?

I would rather not support both key names. For this project, we can instead require specific Bolt version(s). We should always aim to support the newest Bolt version, and we aren't required to support older.

  * previously when getting data after a task is run, we fetched
    the result data.  However, a newer version of bolt changed
    the key name to value and broke this code.  This fixes that
    by fetching the newer key name and failing back to the old
    key name if the value does not exist.
@logicminds
Copy link
Contributor Author

logicminds commented Aug 4, 2020

@reidmv not sure which version. Within the last 2 months. Basically the to_data function was not aligned with the result object so they fixed that.

I refactored my code to not even use the to_data function and instead rely more on the result object's methods instead which should not have changed through the versions.

Also made it more verbose so it is easier to read.

@reidmv reidmv merged commit 6f6f665 into puppetlabs:main Aug 4, 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.

2 participants