Skip to content
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

Allow users to name jobs #23

Open
billy1380 opened this issue Mar 6, 2015 · 6 comments · May be fixed by #25
Open

Allow users to name jobs #23

billy1380 opened this issue Mar 6, 2015 · 6 comments · May be fixed by #25

Comments

@billy1380
Copy link
Contributor

There seems to be a job display name and yet it is always the class name.

Add a method to allow the user to set the job display name rather than having to override the getJobDisplayName in every job

@aozarov
Copy link
Contributor

aozarov commented Mar 6, 2015

Yes, the intent was for one to override getJobDisplayName for returning a value which is different from the default. From a user perspective a Job is immutable but this setter violates that (and in an inconsistent way - only going to have an effect upon Job submission). Also, by overriding (vs setter) one could make the value more "dynamic".

@billy1380
Copy link
Contributor Author

In theory what you are saying is correct; in practice I have 10s of jobs/job classes. For all of them I have to override getJobDisplayName that requires me to add a member variable called name to each, then add a setter and/or a constructor to set that name for the job on creation.

Now that sounds tedious, right... I should have a parent class that I inherit from which has that behaviour, except that I am mandated by the framework to inherit from Job. The alternative is for me to Have a set of superclasses called something like NamedJob or something that extends each of the 6 types of Job<>.

As for the immutability of the Job, again that should be true, however, every change of status that requires the persistence of a job instance re-saves the display name. So even though I have no intention of changing the display names of jobs, the design does not reflect that requirement at least not when it comes to the display name.

@aozarov
Copy link
Contributor

aozarov commented Mar 6, 2015

I don't think any change of job status (or retries) re-persist the job instance. Job information such as status is handled/saved-by JobRecord and the instance is saved by JobInstanceRecord. The former gets updated between invocations and status change but (AFAIK) not the latter.
Also, one should consider calls to setDisplayName on a JobRefernce after job was submitted...
I understand your pain but feel that for a typical use-case overriding (a method or a base-class) would be sufficient. I think the correct approach for providing a better support for display-name would be to provide it as a JobSetting (save it similar to other JobSetting values and use it instead of getClass().getName() if JobRecord has a value for it).

@billy1380
Copy link
Contributor Author

I really like the job setting idea. It fits really well. I will have a play and see what I can come up with. Any recommendations? Feel free to close/reject the previous pull request.

@aozarov
Copy link
Contributor

aozarov commented Mar 6, 2015

No recommendation. Follow the path of existing JobSetting handling (shouldn't be too bad).

@billy1380
Copy link
Contributor Author

:) Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants