[jira] [Commented] (SLING-3169) Naming of Job related enumerations
[
https://issues.apache.org/jira/browse/SLING-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13796522#comment-13796522
]
Carsten Ziegeler commented on SLING-3169:
-
Thanks for the feedback [~sseif...@pro-vision.de]
I've now committed a slightly different solution:
ResultBuilder result();
public interface ResultBuilder {
ResultBuilder message(final String message);
JobExecutionResult succeeded();
JobExecutionResult failed();
JobExecutionResult failed(final long retryDelayInMs);
JobExecutionResult cancelled();
}
CANCELLED vs FAILED: a job consumer result is based on what the job consumer
knows: it knows whether retrying the job does not make sense at all (CANCELLED)
or whether the job could be retried (FAILED) (the names are maybe not perfect
but that's what we picked :) ). When FAILED is signaled by the consumer, the
job engine decides based on the queue configuration (number of retries) whether
the job will be requeued or is marked as permanently FAILED. So a FAILED
returned by the consumer either results in QUEUED or CANCELLED.
And based on SUCCESS, FAILED and CANCELLED we have the notification events
right now.
Now for the enum JobState this is right now for consumers interested in the job
state, we have right now:
enum JobType {
QUEUED,
ACTIVE,
SUCCEEDED,
CANCELLED
};
where SUCCEEDED and CANCELLED are final states and QUEUED, ACTIVE the states
during queuing and processing.
The FAILED state can be detected, if the job is QUEUED or ACTIVE and the retry
count is greather than 0 - but right now we can't detect why a job is in the
CANCELLED state
So right now we don't distinguish between whether a job has been stopped by the
user, was cancelled by the consumer, throw an exception etc.
> Naming of Job related enumerations
> --
>
> Key: SLING-3169
> URL: https://issues.apache.org/jira/browse/SLING-3169
> Project: Sling
> Issue Type: New Feature
> Components: Extensions
>Reporter: Carsten Ziegeler
>Assignee: Carsten Ziegeler
> Fix For: Extensions Event 3.3.0
>
>
> This is a follow up from SLING-3028 based on comments by Stefan Seifert:
> I find the enum name Job.JobType not ideal, because it does not stand of
> a type but for a state of the job. But there is a JobState enum in the
> consumer API package already.
> I find the enum and class names JobState and JobStatus in the consumer
> package not ideal, because they do not stand for a state, but for a job
> result.
>
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (SLING-3169) Naming of Job related enumerations
[
https://issues.apache.org/jira/browse/SLING-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13795564#comment-13795564
]
Stefan Seifert commented on SLING-3169:
---
took some time to think through the new API twist...
* javadocs in JobExecutionResult is wrong, the constants SUCCEEDED, FAILED are
not accessible because in a class in an impl package
* JobExecutionContext: the "constant methods" are strange with the non-standard
naming all-uppercase. i would preface standard headless camel case for them.
* JobExecutionContext: the fluent syntax is not so fluent here for the new
methods. the result message is optional, but the method has be called to reach
the retryDelay method. and the retryDely method can even be called for
successful jobs. i woud prefer methods like this:
{code}
ResultBuilder succeeded();
ResultBuilder failed();
ResultBuilder failedRetry(final long retryDelayInMs);
ResultBuilder cancelled();
public interface ResultBuilder {
ResultBuilder result(final String message);
}
{code}
or just even simpler but less flexible for future extensions:
{code}
void succeeded();
void succeeded(final String message);
void failed();
void failed(final String message);
void failedRetry(final long retryDelayInMs);
void failedRetry(final String message, final long retryDelayInMs);
void cancelled();
void cancelled(final String message);
{code}
* CANCELLED vs. FAILED state: why keeping the distinction in the job result,
but not in the job state? either we should keep this in the jobstate as well,
or do not distinguish it in the job result?
> Naming of Job related enumerations
> --
>
> Key: SLING-3169
> URL: https://issues.apache.org/jira/browse/SLING-3169
> Project: Sling
> Issue Type: New Feature
> Components: Extensions
>Reporter: Carsten Ziegeler
>Assignee: Carsten Ziegeler
> Fix For: Extensions Event 3.3.0
>
>
> This is a follow up from SLING-3028 based on comments by Stefan Seifert:
> I find the enum name Job.JobType not ideal, because it does not stand of
> a type but for a state of the job. But there is a JobState enum in the
> consumer API package already.
> I find the enum and class names JobState and JobStatus in the consumer
> package not ideal, because they do not stand for a state, but for a job
> result.
>
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (SLING-3169) Naming of Job related enumerations
[ https://issues.apache.org/jira/browse/SLING-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13795034#comment-13795034 ] Carsten Ziegeler commented on SLING-3169: - Yepp, all this information is available from the job > Naming of Job related enumerations > -- > > Key: SLING-3169 > URL: https://issues.apache.org/jira/browse/SLING-3169 > Project: Sling > Issue Type: New Feature > Components: Extensions >Reporter: Carsten Ziegeler >Assignee: Carsten Ziegeler > Fix For: Extensions Event 3.3.0 > > > This is a follow up from SLING-3028 based on comments by Stefan Seifert: > I find the enum name Job.JobType not ideal, because it does not stand of > a type but for a state of the job. But there is a JobState enum in the > consumer API package already. > I find the enum and class names JobState and JobStatus in the consumer > package not ideal, because they do not stand for a state, but for a job > result. > -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3169) Naming of Job related enumerations
[ https://issues.apache.org/jira/browse/SLING-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13795027#comment-13795027 ] Bertrand Delacretaz commented on SLING-3169: I was going to say that using QUEUED for new as well as retrying jobs is not precise enough, but a single state might be enough provided there's a way to get more information. Is there a way to get more details about the state of job retries? Being able to find out how many times the job has already been retried, how many retries are left and for how long we've been trying to execute the job come to mind. > Naming of Job related enumerations > -- > > Key: SLING-3169 > URL: https://issues.apache.org/jira/browse/SLING-3169 > Project: Sling > Issue Type: New Feature > Components: Extensions >Reporter: Carsten Ziegeler >Assignee: Carsten Ziegeler > Fix For: Extensions Event 3.3.0 > > > This is a follow up from SLING-3028 based on comments by Stefan Seifert: > I find the enum name Job.JobType not ideal, because it does not stand of > a type but for a state of the job. But there is a JobState enum in the > consumer API package already. > I find the enum and class names JobState and JobStatus in the consumer > package not ideal, because they do not stand for a state, but for a job > result. > -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3169) Naming of Job related enumerations
[ https://issues.apache.org/jira/browse/SLING-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13794967#comment-13794967 ] Carsten Ziegeler commented on SLING-3169: - [~sseif...@pro-vision.de] I'Ve committed my above mentioned changes and we now have a single JobState enumeration with the four values (QUEUED, ACTIVE, SUCCEEDED, CANCELLED). Before discussing the FAILED state, let's see if the current solution is better than the previous one: I've added a JobExecutionResult interface (which is the result of job processing from a job executor) - to avoid the problem that e.g. QUEUED or ACTIVE is returned as the state I've added methods to the JobExecutionContext to create the result object, either it's the simple "constant" result or by using the builder pattern, its possible to add more information. This way, client code newer has to explicitely use the enumeration. Once we are clear about JobState we might want to use that instead of the isXYZ methods in JobExecutionResult WDYT? > Naming of Job related enumerations > -- > > Key: SLING-3169 > URL: https://issues.apache.org/jira/browse/SLING-3169 > Project: Sling > Issue Type: New Feature > Components: Extensions >Reporter: Carsten Ziegeler >Assignee: Carsten Ziegeler > Fix For: Extensions Event 3.3.0 > > > This is a follow up from SLING-3028 based on comments by Stefan Seifert: > I find the enum name Job.JobType not ideal, because it does not stand of > a type but for a state of the job. But there is a JobState enum in the > consumer API package already. > I find the enum and class names JobState and JobStatus in the consumer > package not ideal, because they do not stand for a state, but for a job > result. > -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3169) Naming of Job related enumerations
[
https://issues.apache.org/jira/browse/SLING-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13794536#comment-13794536
]
Stefan Seifert commented on SLING-3169:
---
ok, lets have a look on the current implementation:
{code}
enum JobType {
QUEUED,
ACTIVE,
SUCCEEDED,
CANCELLED
};
public enum JobState {
SUCCEEDED, // processing finished successfully
FAILED, // processing failed, can be retried
CANCELLED // processing failed permanently
}
{code}
a new, merged enum could look like this:
{code}
enum JobState {
QUEUED, // waiting in queue after adding or for restart after failing
ACTIVE, // job is currently processed
SUCCEEDED, // processing finished successfully
CANCELLED, // processing was canceled during processing
FAILED // processing failed even after the number of configured retries
};
{code}
please note that i slightly redefined some of the states, this can be
questioned of course. first of all i think the merging is a good option, it
removes the partially redundancy betweend the two enums and solves the naming
clash problem in an elegant way. i thought it was usful for a reporting
perpective of history jobs to have a distinction whether a job was failed due
to errors, or because it was interrupted by human or other intraction via the
stopJobById method.
but the merging has one problem: the enum is used as part of the JobStatus
class to allow the job reporting back its status from the JobConsumer point of
view. then it would be possible for the consumer to report the status "QUEUED"
or "ACTIVE" - which should not be allowed. this could be handled by ignoring it
and logging an error, but still is not elegant. perhaps reducing the JobStatus
succeeded feedback to a boolean true/false? the job engine knows if it forced a
stopping/canceling of the job or not. (but to be precise it does not know if
the stop event was ignored by the job consumer and the job perhaps succeeded
anyway)
> Naming of Job related enumerations
> --
>
> Key: SLING-3169
> URL: https://issues.apache.org/jira/browse/SLING-3169
> Project: Sling
> Issue Type: New Feature
> Components: Extensions
>Reporter: Carsten Ziegeler
>Assignee: Carsten Ziegeler
> Fix For: Extensions Event 3.3.0
>
>
> This is a follow up from SLING-3028 based on comments by Stefan Seifert:
> I find the enum name Job.JobType not ideal, because it does not stand of
> a type but for a state of the job. But there is a JobState enum in the
> consumer API package already.
> I find the enum and class names JobState and JobStatus in the consumer
> package not ideal, because they do not stand for a state, but for a job
> result.
>
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (SLING-3169) Naming of Job related enumerations
[ https://issues.apache.org/jira/browse/SLING-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13794375#comment-13794375 ] Carsten Ziegeler commented on SLING-3169: - Or maybe we don't need the FAILED state at all - QUEUED/ACTIVE in combination with getRetryCount() gives the same information > Naming of Job related enumerations > -- > > Key: SLING-3169 > URL: https://issues.apache.org/jira/browse/SLING-3169 > Project: Sling > Issue Type: New Feature > Components: Extensions >Reporter: Carsten Ziegeler >Assignee: Carsten Ziegeler > Fix For: Extensions Event 3.3.0 > > > This is a follow up from SLING-3028 based on comments by Stefan Seifert: > I find the enum name Job.JobType not ideal, because it does not stand of > a type but for a state of the job. But there is a JobState enum in the > consumer API package already. > I find the enum and class names JobState and JobStatus in the consumer > package not ideal, because they do not stand for a state, but for a job > result. > -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3169) Naming of Job related enumerations
[ https://issues.apache.org/jira/browse/SLING-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13793629#comment-13793629 ] Carsten Ziegeler commented on SLING-3169: - I agree this is a little bit tricky (and confusing) - let's start with the obvious one, Job.JobType. I agree this is a state and I choose this name because there is JobState in the consumer package :) Now JobState in the consumer package is more a result, but not quiet. One could argue that the result of a job is whatever it produced, so the result of an image rendering job is the newly rendered image. Subtle differences I agree, so what about renaming JobStatus to JobExecutionResult? (to distinguish it from a job result) We could think about merging JobState and Job.JobType - however there is the problem with FAILED vs QUEUED, QUEUED would mean it has never been executed and is in the queue, FAILED means it has been executed but processing failed and the job is now in the queue. [~sseif...@pro-vision.de] WDYT? > Naming of Job related enumerations > -- > > Key: SLING-3169 > URL: https://issues.apache.org/jira/browse/SLING-3169 > Project: Sling > Issue Type: New Feature > Components: Extensions >Reporter: Carsten Ziegeler >Assignee: Carsten Ziegeler > Fix For: Extensions Event 3.3.0 > > > This is a follow up from SLING-3028 based on comments by Stefan Seifert: > I find the enum name Job.JobType not ideal, because it does not stand of > a type but for a state of the job. But there is a JobState enum in the > consumer API package already. > I find the enum and class names JobState and JobStatus in the consumer > package not ideal, because they do not stand for a state, but for a job > result. > -- This message was sent by Atlassian JIRA (v6.1#6144)
