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

Fixes incorrect timestamp calculation reported in aodn/content/416 #714

Merged
merged 2 commits into from
Jun 27, 2019

Conversation

jonescc
Copy link
Contributor

@jonescc jonescc commented May 3, 2019

for anmn_ts_timeseries harvester

Problem occurred when using Calendar instance to round to nearest second. There was an associated time zone of AEDT which got converted to AEST when adding 500ms for some reason.

Remove dependency on netcdf library for performing this calculation.

Add unit testing code to routine.

@jonescc
Copy link
Contributor Author

jonescc commented May 3, 2019

Note: this routine is currently included in each harvester that uses it. The fix here just updates that code, but its also possible to rewrite this routine as a plugin system routine so the code would only need to be fixed in one place. It would then be copied to the harvester whenever it was opened/built. Need a backlog item for that as there is some work involved.

@lbesnard
Copy link
Contributor

@jonescc how do we test/deploy this via jenkins since it affects a lot of harvesters and will blow away the limit of 25 or so of harvester pipelines to be displayed on jenkins?

@jonescc
Copy link
Contributor Author

jonescc commented May 16, 2019

I'll check, but I can't see why there needs to be a limit anymore - all the artifacts are being stored on s3 now.

@jonescc
Copy link
Contributor Author

jonescc commented May 16, 2019

The alternative is separate pull requests for each harvester.

@jonescc
Copy link
Contributor Author

jonescc commented May 16, 2019

@lwgordonimos, any reason why we can't increase the number of builds kept for talend jobs now? Its currently set to 40. I've set the talend 7 build jobs to retain up to 200 without any issues.

@ghost
Copy link

ghost commented May 16, 2019

@jonescc no reason any more since the build artifacts are now 100% S3. Storing a single build is now <200kB on the Jenkins server, so should be fine!

@jonescc
Copy link
Contributor Author

jonescc commented May 17, 2019

@lbesnard - jenkins has been modified to display up to 200 previous builds.

@lbesnard
Copy link
Contributor

thanks. ill wait for all the po's to be back next week before merging this so we can push to prod the previous versions first

@lbesnard
Copy link
Contributor

@jonescc can we rebase this PR against master ? or shall we wait until all TOS 7 harvesters are on prod ?

@jonescc
Copy link
Contributor Author

jonescc commented Jun 13, 2019

Will need to rebase against the tos 7 branch which will eventually be merged into master. No point doing it in master as the code it changes is going to change.

…r anmn_ts_timeseries harvester

Problem occurred when using Calendar instance to round to nearest second.  There was an associated time zone of AEDT which got converted to AEST when adding 500ms for some reason.

Remove dependency on netcdf library for performing this calculation.

Add unit testing code to routine.
@jonescc
Copy link
Contributor Author

jonescc commented Jun 27, 2019

Rebased against master now that tos-7-1-1 has been merged - should be OK to go when ready.

@lbesnard lbesnard merged commit cd6aac3 into master Jun 27, 2019
@lbesnard lbesnard deleted the content-issue-416 branch June 27, 2019 06:57
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 this pull request may close these issues.

2 participants