Design of methods with XXXOptions

[shawkins]

This topic originated on #3316. list/watch both support ListOptions as an argument. Delete does not, but does support quite a few context methods.

It is not clear if delete should also support delete(DeleteOptions) for consistency with the list/watch methods.

There is also a consideration of what passing options directly makes sense given the usage of conflicting context values.

For example if you do:

client.configMaps().withField("y", "z").list(new ListOptionsBuilder().withFieldSelector("x=y").build())

It will currently send both:

GET /api/v1/namespaces/test/configmaps?fieldSelector=x%3Dy&fieldSelector=y%3Dz

An alternative design to directly passing the options is to inline the fluent construction, similar to what is being done for filtering:

public interface DeleteOptionsNested<T> extends DeleteOptionsFluent<DeleteOptionsNested<T>>, Nested<T> {
  
  default T endDeleteOptions() {
    return and();
  }
  
}
...
public interface Deletable {

  ...
  DeleteOptionsNested<Deletable> withDeleteOptions();

}

That allows for calls like:

client.configMaps().withName("map1").withDeleteOptions().withNewPreconditions("800", null).and().delete()

The understanding here is that the constructed deleted options will be the ones passed to the operation. If you override the default value passed in from the context, then it is explicit what you want to have happen.

The implementation with inline fluents is straight-forward. The context needs a reference to DeleteOptions and an extended DeleteFluentImpl to maintain a reference to the operation so that it can be returned with the updated context when the fluent nested ends.

Any property that is specific to a given operation (gracePeriodSeconds, propagationPolicy, cascading) would just move to a DeleteOptions on the context, rather than having an individual value. However common things like dryRun and this locked resourceVersion don't map neatly and would still need placeholders in the context and would presumably influence the default values of the DeleteOptions returned by withDeleteOptions.

@rohanKanojia @manusa WDYT:

• should delete support all options?
• should we stick with passing options or inline their construction?
• if we stick with passing in the options, should we clarify what the relationship is between the existing context values and those in the options?

[shawkins]

After some discussion it seems preferred not to further complicate the context (with) methods.

Another option is to use methods similar to edit:

delete(UnaryOperation<DeleteOptions> options)
list(UnaryOperation<ListOptions> options)
watch(UnaryOperation<ListOptions> options)

The options passed into the function will be populated already with values from the context. Anything done to manipulate them further will be what is passed to the api call. We can optionally consider using the signature:

delete(Consumer<DeleteOptionsBuilder> options)

Which would be more concise for the recommended usage pattern, but more restrictive.

The methods directly passing in ListOptions will be deprecated.

The context withXXX methods that apply to only one operation, such as withPropagationPolicy, could be deprecated - just to minimize what needs to be exposed.

@metacosm @manusa @rohanKanojia do you prefer the UnaryOperation or the Builder form of these functions?