Fix breakage caused by external use of our internal API #953
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Because Logstash Core v7.7.0-v7.8.x override the `use_event_type?` in a monitoring implelentation's subclass with a required (and ignored) `client` argument, when we send `use_event_type?` internally, we must send an argument or else we break compatibility with those versions of Logstash. This change re-adds the required argument and modifies our call-site to send with an arity of 1, re-names the resulting local variable to be descriptive of our intent, and adds commentary to prevent future accidental breakage.
|
Not sure how or why, but I've pushed changelog and version bump to the branch referenced here and Github is not picking it up -> master...yaauie:fix-external-use-of-internal-api |
|
GitHub is acting up, already happened in another PR
…On Wed, 15 Jul 2020 at 17:57, Ry Biesemeyer ***@***.***> wrote:
Not sure how or why, but I've pushed changelog and version bump to the
branch referenced here and Github is not picking it up ->
master...yaauie:fix-external-use-of-internal-api
<master...yaauie:fix-external-use-of-internal-api>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#953 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAHYQPXM7PCD2L62ENYAPTR3XNYRANCNFSM4O2WPSEA>
.
|
yaauie
force-pushed
the
fix-external-use-of-internal-api
branch
from
5dee3ba to
baeb4b1
Compare
|
Ah, it seems caught up now. |
jsvd
approved these changes
Jul 15, 2020
| @@ -60,7 +60,14 @@ def successful_connection? | |||
| !!maximum_seen_major_version | |||
| end | |||
|
|
|||
| def use_event_type? | |||
| ## | |||
| # WARNING: This method is overridden in a subclass in Logstash Core 7.7-7.8's monitoring, | |||
|
Needs a doc fix too before publishing. |
yaauie
force-pushed
the
fix-external-use-of-internal-api
branch
from
a14e5da to
fb9657c
Compare
karenzone
reviewed
Jul 15, 2020
Versioned Plugins doc gen doesn't have the placeholder, so using it breaks the build. Co-authored-by: Karen Metts <35154725+karenzone@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because Logstash Core v7.7.0-v7.8.x override the
use_event_type?in amonitoring implelentation's subclass with a required (and ignored)
clientargument, when we send
use_event_type?internally, we must send an argumentor else we break compatibility with those versions of Logstash.
This change re-adds the required argument and modifies our call-site to send
with an arity of 1, re-names the resulting local variable to be descriptive
of our intent, and adds commentary to prevent future accidental breakage.