You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2021/09/02 21:45:13 UTC

[GitHub] [jena] Aklakan opened a new pull request #1063: JENA-2154: Custom SERVICE executors

Aklakan opened a new pull request #1063:
URL: https://github.com/apache/jena/pull/1063


   The implementation introduces a ServiceExecutorRegistry akin to the existing FunctionRegistry.
   Motivation and design are outlined at https://issues.apache.org/jira/browse/JENA-2154.
   Test cases are provided in the TestCustomServiceExecutor class.
   
   Cheers,
   Claus


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan edited a comment on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan edited a comment on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-913922074


   I agree with most points - but one remark:
   
   The rationale for the deferred start is mainly to allow for showing a list of applicable service executors for a given query - e.g. using a debug function or an admin web interface - without actually starting the execution. Returning the QueryIterator directly does not guarantee that; same iterators may already be live when they are returned - others only initialize on .next / .hasNext. 
   As a custom service clause is essentially a specification of some task, createExecutor should essentially take the role of validating the OpService (raise an exception if something is wrong, like illegal arguments) and if all is fine then return the object from which the execution can be started.
   
   So with the supplier I wanted to avoid to introduce yet another interface, but I actually had that in mind:
   
   ```java
   public interface ServiceExecutorFactory {
       ServiceExecution createExecutor(OpService op, OpService original, Binding binding, ExecutionContext context);
   }
   
   public interface ServiceExecution { // Similar to QueryExecution
       QueryIterator exec(); // Similar to execSelect
   }
   ```
   
   Yet, it does not exactly match up, because the QueryIterator has the close() method (unlike ResultSet).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan edited a comment on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan edited a comment on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-913922074


   I agree with most points - but one remark:
   
   The rationale for the deferred start is mainly to allow for showing a list of applicable service executors for a given query - e.g. using a debug function or an admin web interface - without actually starting the execution. Returning the QueryIterator directly does not guarantee that; same iterators may already be live when they are returned - others only initialize on .next / .hasNext. 
   As a custom service clause is essentially a specification of some task, createExecutor should essentially take the role of **validating** the OpService (raise an exception if something is wrong, like illegal arguments). Only if all is fine then return the object from which the execution can be started. I'd say this is an important aspect in the architecture. As mentioned, this could also be a separate method - but then the code typically becomes:
   ```java
   if (factory.validate(args)) {
     factory.run(args); //  Usually calls validate as first thing again
   }
   ```
   
   So with the supplier I wanted to avoid to introduce yet another interface, but I actually had that in mind:
   
   ```java
   public interface ServiceExecutorFactory {
       ServiceExecutor createExecutor(OpService op, OpService original, Binding binding, ExecutionContext context);
   }
   
   public interface ServiceExecutor { // Similar to QueryExecution
       QueryIterator exec(); // Similar to execSelect
   }
   ```
   
   Yet, it does not exactly match up, because in QueryExecution/ResultSet vs ServiceExecutor/QueryIterator the QueryIterator is the actual execution (it also has the close() method which aborts execution).
   
   I realize that some time ago we also discussed the use case where SERVICE is invoked for batches of bindings (possibly in parallel) by using a different base class than QueryIterRepeatApply. The current design would not yet account for that. The main question here is whether this would be a separate architecture anyway. I guess so.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan edited a comment on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan edited a comment on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-913922074


   I agree with most points - but one remark:
   
   The rationale for the deferred start is mainly to allow for showing a list of applicable service executors for a given query - e.g. using a debug function or an admin web interface - without actually starting the execution. Returning the QueryIterator directly does not guarantee that; some iterators may already be live when they are returned - others only initialize on .next / .hasNext. 
   As a custom service clause is essentially a specification of some task, createExecutor should essentially take the role of **validating** the OpService (raise an exception if something is wrong, like illegal arguments). Only if all is fine then return the object from which the execution can be started. I'd say this is an important aspect in the architecture. As mentioned, this could also be a separate method - but then the code typically becomes:
   ```java
   if (factory.validate(args)) {
     factory.run(args); //  Usually calls validate as first thing again
   }
   ```
   
   So with the supplier I wanted to avoid to introduce yet another interface, but I actually had that in mind:
   
   ```java
   public interface ServiceExecutorFactory {
       ServiceExecutor createExecutor(OpService op, OpService original, Binding binding, ExecutionContext context);
   }
   
   public interface ServiceExecutor { // Similar to QueryExecution
       QueryIterator exec(); // Similar to execSelect
   }
   ```
   
   Yet, it does not exactly match up, because in QueryExecution/ResultSet vs ServiceExecutor/QueryIterator the QueryIterator is the actual execution (it also has the close() method which aborts execution).
   
   I realize that some time ago we also discussed the use case where SERVICE is invoked for batches of bindings (possibly in parallel) by using a different base class than QueryIterRepeatApply. The current design would not yet account for that. The main question here is whether this would be a separate architecture anyway. I guess so.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan commented on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan commented on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-913922074


   I agree with most points - but one remark:
   
   The rationale for the deferred start is mainly to allow for showing a list of applicable service executors for a given query - e.g. using a debug function or an admin web interface - without actually starting the execution. Returning the QueryIterator directly does not guarantee that; same iterators may already be live when they are returned - others only initialize on .next / .hasNext. 
   As a custom service clause is essentially a specification of some task, createExecutor should essentially take the role of validating the OpService (raise an exception if something is wrong, like illegal arguments) and if all is fine then return the object from which the execution can be started.
   
   So with the supplier I wanted to avoid to introduce yet another interface, but I actually had that in mind:
   
   ```java
   public interface ServiceExecutorFactory {
       ServiceExecution createExecutor(OpService op, OpService original, Binding binding, ExecutionContext context);
   }
   
   public interface ServiceExecution { // Similar to QueryExecution
       QueryIterator exec(); // Similar to execSelect
   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs edited a comment on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
afs edited a comment on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-916906512


   `ServiceExecutor` works for me. Maybe `ServiceExecution` c.f. `QueryExecution`. Executor is, to me, then thing that does work, may called several times, not the outcome of the work.
   
   Both are better than `Supplier` which does not say anything about calling `get()` multiple times (same object? new object each time?) . Jena has `Creator` for that role.
   
   I'll make the changes - due to timing, 4.2.0 needs to get out soon and I have a window of time to do the release; we can refine this as necessary
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs closed pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
afs closed pull request #1063:
URL: https://github.com/apache/jena/pull/1063


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan edited a comment on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan edited a comment on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-913922074


   I agree with most points - but one remark:
   
   The rationale for the deferred start is mainly to allow for showing a list of applicable service executors for a given query - e.g. using a debug function or an admin web interface - without actually starting the execution. Returning the QueryIterator directly does not guarantee that; same iterators may already be live when they are returned - others only initialize on .next / .hasNext. 
   As a custom service clause is essentially a specification of some task, createExecutor should essentially take the role of **validating** the OpService (raise an exception if something is wrong, like illegal arguments). Only if all is fine then return the object from which the execution can be started. I'd say this is an important aspect in the architecture. As mentioned, this could also be a separate method - but then the code typically becomes:
   ```java
   if (factory.validate(args)) {
     factory.run(args); //  Usually calls validate as first thing again
   }
   ```
   
   So with the supplier I wanted to avoid to introduce yet another interface, but I actually had that in mind:
   
   ```java
   public interface ServiceExecutorFactory {
       ServiceExecutor createExecutor(OpService op, OpService original, Binding binding, ExecutionContext context);
   }
   
   public interface ServiceExecutor { // Similar to QueryExecution
       QueryIterator exec(); // Similar to execSelect
   }
   ```
   
   Yet, it does not exactly match up, because in QueryExecution/ResultSet vs ServiceExecutor/QueryIterator the QueryIterator is the actual execution (it also has the close() method which aborts execution).
   
   I realize that time ago we also discussed the use case where SERVICE is invoked for batches of bindings; the current design would not yet account for that. Then again, maybe a more powerful system could be added later that can delegate to this simpler one?!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
afs commented on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-912978339


   This is looking like good timing. While I, personally, think using SERVICE for extensions for stored procedures isn't perfect, it does stay within standard syntax which is valued by users and has some take-up.
   
   Thanks for following existing design patterns (even if they are, with hindsight a bit overly complex in places!).
   
   Starting with design review:
   
   _Deferred start_
   
   Delayed initialization can handled orthogonally. See
   `IteratorDelayedInitialization` for example. `Supplier` is coping with an uncommon case
   that I think can be done within an implementation anyway.
   
   _Location_
   
   > Probably it is indeed better to move the code from QueryIterService to Service - but add the 'Binding' argument to the function in Service
   
   Possible, though I don't think it make much difference either way.  Leave it in `QueryIterService` for now?
   
   _Default_
   
   I'd like to handle the default case (in the PR, no factory found) as a regular `ServiceExecutorFactory` added from `ARQ.init` rather than code here. It is nice to have a "really no SERVICE support" done by clearing the registry even if we keep the old context approach.
   
   The code will need to handle "no factory found" as `QueryExecException` or a join-identity
   query iterator depending on `SILENT`.
   
   _Substituted vs original op_
   
   This isn't so easy to treat as separate.
   
   * A dispatch may be `SERVICE ?service` with `?service` already bound but the
   registration is to `<someURI>`.
   
   * This code is in `nextStage` so iteration is happening further out. If the implementation wants bottom-up only, might have to have a different optimizer - i.e. needs to take a wider view. It can still do a single execution and keep that internally (`QueryIterator.close` will be called for clear-up).
   
   I don't think that is common so the API can focus on the usual case because the other can still be done as long as the original is available.
   
   How about providing both in the factory call so the implementation is informed?
   
   With those points:
   
   ```java
   public interface ServiceExecutorFactory {
       QueryIterator createExecutor(OpService op, OpService original, Binding binding, ExecutionContext context);
   }
   ```
   
   The switch to `ExecutionContext` is a good idea.
   
   _Timing_
   
   We are close to the 4.2.0 release point. As an extension to advertise, we should take the time to get it right. A whole cycle (3-4 months) is a bit long so my suggestion is include in 4.2.0 with the understanding that it is "Experimental - work-in-progress - subject to refinement".
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
afs commented on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-913746300


   Suggested changes made to https://github.com/afs/jena/tree/service (including moving tests to jena-integration as per an old comment). The branch is based off the PR commits.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
afs commented on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-916906512


   `ServiceExecutor` works for me.
   
   Better than `Supplier` which does not say anything about calling `get()` multiple times (same object? new object each time?) . Jena has `Creator` for that role.
   
   But a specifically named `ServiceExecutor` is good.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
afs commented on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-917419606


   Included in PR #1070 .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan edited a comment on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan edited a comment on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-913922074


   I agree with most points - but one remark:
   
   The rationale for the deferred start is mainly to allow for showing a list of applicable service executors for a given query - e.g. using a debug function or an admin web interface - without actually starting the execution. Returning the QueryIterator directly does not guarantee that; same iterators may already be live when they are returned - others only initialize on .next / .hasNext. 
   As a custom service clause is essentially a specification of some task, createExecutor should essentially take the role of **validating** the OpService (raise an exception if something is wrong, like illegal arguments). I'd say this is an important aspect in the architecture. As mentioned, this could also be a separate method - but then the code is typically 
   ```java
   if (factory.validate(args)) {
     factory.run(args); //  Usually calls validate as first thing again
   }
   ```
   Only if all is fine then return the object from which the execution can be started.
   
   So with the supplier I wanted to avoid to introduce yet another interface, but I actually had that in mind:
   
   ```java
   public interface ServiceExecutorFactory {
       ServiceExecutor createExecutor(OpService op, OpService original, Binding binding, ExecutionContext context);
   }
   
   public interface ServiceExecutor { // Similar to QueryExecution
       QueryIterator exec(); // Similar to execSelect
   }
   ```
   
   Yet, it does not exactly match up, because in QueryExecution/ResultSet vs ServiceExecutor/QueryIterator the QueryIterator is the actual execution (it also has the close() method which aborts execution).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan edited a comment on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan edited a comment on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-913922074


   I agree with most points - but one remark:
   
   The rationale for the deferred start is mainly to allow for showing a list of applicable service executors for a given query - e.g. using a debug function or an admin web interface - without actually starting the execution. Returning the QueryIterator directly does not guarantee that; same iterators may already be live when they are returned - others only initialize on .next / .hasNext. 
   As a custom service clause is essentially a specification of some task, createExecutor should essentially take the role of validating the OpService (raise an exception if something is wrong, like illegal arguments) and if all is fine then return the object from which the execution can be started.
   
   So with the supplier I wanted to avoid to introduce yet another interface, but I actually had that in mind:
   
   ```java
   public interface ServiceExecutorFactory {
       ServiceExecutor createExecutor(OpService op, OpService original, Binding binding, ExecutionContext context);
   }
   
   public interface ServiceExecutor { // Similar to QueryExecution
       QueryIterator exec(); // Similar to execSelect
   }
   ```
   
   Yet, it does not exactly match up, because in QueryExecution/ResultSet vs ServiceExecutor/QueryIterator the QueryIterator is the actual execution (it also has the close() method which aborts execution).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan edited a comment on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan edited a comment on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-913922074


   I agree with most points - but one remark:
   
   The rationale for the deferred start is mainly to allow for showing a list of applicable service executors for a given query - e.g. using a debug function or an admin web interface - without actually starting the execution. Returning the QueryIterator directly does not guarantee that; same iterators may already be live when they are returned - others only initialize on .next / .hasNext. 
   As a custom service clause is essentially a specification of some task, createExecutor should essentially take the role of **validating** the OpService (raise an exception if something is wrong, like illegal arguments). Only if all is fine then return the object from which the execution can be started. I'd say this is an important aspect in the architecture. As mentioned, this could also be a separate method - but then the code typically becomes:
   ```java
   if (factory.validate(args)) {
     factory.run(args); //  Usually calls validate as first thing again
   }
   ```
   
   So with the supplier I wanted to avoid to introduce yet another interface, but I actually had that in mind:
   
   ```java
   public interface ServiceExecutorFactory {
       ServiceExecutor createExecutor(OpService op, OpService original, Binding binding, ExecutionContext context);
   }
   
   public interface ServiceExecutor { // Similar to QueryExecution
       QueryIterator exec(); // Similar to execSelect
   }
   ```
   
   Yet, it does not exactly match up, because in QueryExecution/ResultSet vs ServiceExecutor/QueryIterator the QueryIterator is the actual execution (it also has the close() method which aborts execution).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan edited a comment on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan edited a comment on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-912442963


   A couple of remarks which you probably want to comment on:
   
   * The rationale for the supplier indirection is to defer starting the actual execution. This - in principle - allows for checking which of the executor factories could handle the arguments without immediately starting the execution.
   ```java
   class ServiceExecutorFactory {
     Supplier<QueryIterator> createExecutor(OpService op, Binding binding, ExecutionContext execCxt);
   }
   ```
   One could also use separate 'boolean canExecute()' and 'execute()' methods, however, with that pattern it sometimes happens that `canDoX()` and `doX()` need to internally perform redundant computations.
   
   * Probably it is indeed better to move the code from QueryIterService to Service - but add the 'Binding' argument to the function in Service
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan commented on pull request #1063: JENA-2154: Custom SERVICE executors

Posted by GitBox <gi...@apache.org>.
Aklakan commented on pull request #1063:
URL: https://github.com/apache/jena/pull/1063#issuecomment-912442963


   A couple of remarks which you probably want to comment on:
   
   * The rationale for the supplier indirection is to defer starting the actual execution. This - in principle - allows for checking which of the executor factories could handle the arguments without immediately starting the execution.
   ```java
   class ServiceExecutorFactory {
     Supplier<QueryIterator> createExecutor(OpService op, Binding binding, ExecutionContext execCxt);
   }
   ```
   * Probably it is indeed better to move the code from QueryIterService to Service - but add the 'Binding' argument to the function in Service
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org