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 upgrade without replica #198

Merged
merged 3 commits into from
Aug 18, 2021
Merged

Fix upgrade without replica #198

merged 3 commits into from
Aug 18, 2021

Conversation

reidmv
Copy link
Contributor

@reidmv reidmv commented Aug 18, 2021

Dan found this bug and commented in Slack. The new test shows the failing situation. The proposed fix is in this draft PR.

reidmv added 3 commits August 18, 2021 10:22
Test for when a primary, and a compiler are both given, but no replica
is given
Previously, the $target.name was used as the hash key. This was
originally done to work around a bug in an older version of Bolt; it is
not necessary to use the .name string as the hash key.

Using .name makes it more difficult to deal with when there are no
actual Target objects in an array. So let's stop doing that.
This fixes the situation where there is no replica, so computing the
second compiler group cannot be done due to hitting an array indexing
issue. By switching to dig, undef will simply be returned in that
situation.
@reidmv reidmv force-pushed the fix-upgrade-without-replica branch from 33a1a1e to 8ef9bd5 Compare August 18, 2021 17:22
@reidmv reidmv marked this pull request as ready for review August 18, 2021 17:23
@reidmv reidmv requested a review from a team as a code owner August 18, 2021 17:23
Copy link
Member

@ody ody left a comment

Choose a reason for hiding this comment

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

Code only contains a single minor logic modification, else just swapping out functions. Think this would have a low potential for causing issues and fixes a bug so seems good to me. Spec test seems straight forward but I've not had the opportunity to get very familiar with spec testing plans so have no substantive feedback on it.

@reidmv reidmv merged commit 42b730a into main Aug 18, 2021
@reidmv reidmv deleted the fix-upgrade-without-replica branch August 18, 2021 18:49
@reidmv reidmv added the bugfix label Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants