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

Improve DateTime resolution for job arguments #109

Merged
merged 3 commits into from Jun 1, 2014
Merged

Improve DateTime resolution for job arguments #109

merged 3 commits into from Jun 1, 2014

Conversation

dennyferra
Copy link
Contributor

At job execution if a DateTime argument was included the DateTime object does not include milliseconds. The issue is caused at job creation when converting arguments to their string representation. The default conversion does not provide milliseconds. This change is backwards compatible with existing data since a new DateTime object can be instantiated with both the previous format and this new format, example:

old  "05/30/2014 07:27:34"
new  "05/30/2014 07:27:34.2791"

If there is a better approach I'd be happy to make the change.

At job execution if a DateTime argument was included the DateTime object does not include milliseconds. The issue is caused at job creation when converting arguments to their string representation. The default conversion does not provide milliseconds. This is backwards compatible with existing data since a new DateTime object can be instantiated with both the old format and this new format, example:

old: "05/30/2014 07:27:34"
new: "05/30/2014 07:27:34.2791"

If there is a better approach I'd be happy to make the change.
@odinserj
Copy link
Member

Thanks for contribution! Can you write unit test to cover this feature?

@dennyferra
Copy link
Contributor Author

Added unit test

@odinserj
Copy link
Member

Please, add also a test, that checks DateTime deserialization is being performed correctly. As I know, there is an example with IJobCancellationToken arguments.

@dennyferra
Copy link
Contributor Author

Added additional test for deserialization of the new DateTime format.

@odinserj
Copy link
Member

odinserj commented Jun 1, 2014

@dennyferra, thank you!

odinserj added a commit that referenced this pull request Jun 1, 2014
Improve DateTime resolution for job arguments
@odinserj odinserj merged commit caeaf17 into HangfireIO:master Jun 1, 2014
@odinserj odinserj added this to the v0.9.0 milestone Jun 1, 2014
@dennyferra dennyferra deleted the patch-1 branch June 5, 2014 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants