-
Notifications
You must be signed in to change notification settings - Fork 24.6k
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
Deprecate async
replication
#10642
Deprecate async
replication
#10642
Conversation
async
replicationasync
replication
async
replicationasync
replication
Thanks for openinig this @dadoonet , it LGTM. As for deprecation warnings, I think they should be warn, but let's hear what @clintongormley thinks. It would be great to have all of the deprecation logs from the same logger, so that they can all be enabled at once when needed, but this might require some more work and overlap with other work in progress that I'm not sure about. |
Ah! So we could create a DeprecationLogger class and allow a global activation or not. Would be super nice indeed! |
I think the deprecation message should be a warning, and I agree with #10642 (comment) (see #8963) |
@dadoonet any movement here? |
@clintongormley I was wondering if I should wait for #11033 to be merged in so I can update the deprecation notice there? |
Hmm, actually ParsedField might not be applicable here (or in an awkward way) since we do not perform parsing, however I think this PR should use #11033 once it's in. |
@dadoonet can you move this forward if possible? |
Actually, the deprecation logger has been merged only in master (2.0) but is not available in 1.x. |
d61f0d2
to
cbd2b6c
Compare
public BulkRequest replicationType(ReplicationType replicationType) { | ||
this.replicationType = replicationType; | ||
return this; | ||
} | ||
|
||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add also what the method does, although quite self-explaining
LGTM besides the tiny javadoc comment I left |
cbd2b6c
to
f2ed5d1
Compare
Follow up for PR elastic#10171. As we discussed elastic#10114 (comment), we should also mark the code as deprecated so plugin developers know in advance that they need to adapt their code.
f2ed5d1
to
1493cfb
Compare
merged in 1.x with 1493cfb |
async
replicationasync
replication
Follow up for PR #10171.
As we discussed #10114 (comment), we should also mark the code as deprecated so plugin developers know in advance that they need to adapt their code.
I also added a
log
when usingreplication
parameter in REST APIs but asinfo
Level. I wonder if I should better log that as aWARN
ifdebug
level is set? So people in debug mode will see this information as aWARN
while it won't pollute logs for each call when in defaultINFO
log level.