[prev in list] [next in list] [prev in thread] [next in thread] 

List:       lucene-dev
Subject:    Re: Accessibility of SegmentInfo::setDiagnostics
From:       Chris Hegarty <christopher.hegarty () elastic ! co ! INVALID>
Date:       2021-09-27 7:07:22
Message-ID: 9AA09331-3352-406E-9ADC-4B88AC5F1954 () elastic ! co
[Download RAW message or body]

Hi Adrien,

Thanks for your reply.

A further thought... the use-case here is really for custom subclasses
to "add additional" diagnostics, rather than "replace". How about we
expose just that functionality in SegmentInfo, e.g. through a new method
similar to:

  /**
   * Adds the given {@code diagnostics} to this segment's diagnostics.
   * @param diagnostics the diagnostics to add
   */
  public void addDiagnostics(Map<String, String> diagnostics) { ... }

This would satisfy the ES use-case, while disallowing a full replace.

If we think that there are other use-cases that require "replace" (or it
is preferred to defer API discussion for a later time), then I'm happy
to proceed with simply making the existing `setDiagnostics` method
public.

-Chris.

> On 24 Sep 2021, at 17:30, Adrien Grand <jpountz@gmail.com> wrote:
> 
> I'd +1 a change that makes setDiagnostics public. Longer term I wonder if we should \
> have a more locked down API that _only_ allows setting diagnostics. There are lots \
> of things in SegmentCommitInfo that merges should never override like the segment \
> ID, and I can't think of anything else than diagnostics that a merge policy should \
> ever need to set. 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[prev in list] [next in list] [prev in thread] [next in thread]