OpenDataCubePipelines / tesp

OpenDataCubePipeline code for the processing of Australasian Regional Copernicus Data Hub L1C S2A/B data
3 stars 6 forks source link

Log summaries #73

Closed sixy6e closed 4 years ago

sixy6e commented 4 years ago

A bunch of changes that includes:

The job also incorporates indexing to an opendatacube database instance (optional on whether to index)

dunkgray commented 4 years ago

Actually that would be practical. Consider changing the name to add indexing --index-datacube-env maybe. I like it. The help info can describe what it does.


From: Josh Sixsmith notifications@github.com Sent: Thursday, 2 July 2020 4:57 PM To: OpenDataCubePipelines/tesp tesp@noreply.github.com Cc: Gray Duncan Duncan.Gray@ga.gov.au; Review requested review_requested@noreply.github.com Subject: Re: [OpenDataCubePipelines/tesp] Log summaries (#73)

@sixy6e commented on this pull request.


In bin/ard_pbshttps://github.com/OpenDataCubePipelines/tesp/pull/73#discussion_r448787534:

@@ -212,19 +293,25 @@ def _submit_summary(indir, outdir, batch_id, pbs_resources, env, job_ids, test): help="Job walltime in hh:mm:ss format.") @click.option("--email", default="", help="Notification email address.") +@click.option("--datacube-env", type=click.Path(exists=True, readable=True),

A cleaner way might be to only have the --datacube-env option, and if set it is then implicit that indexing is to occur. This would remove one command switch, and avoid the additional complexity of dealing with two optional but dependent switches.

What do you think?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHubhttps://github.com/OpenDataCubePipelines/tesp/pull/73#discussion_r448787534, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAIAHKPLLUKJZGB2V66TP2LRZQVV7ANCNFSM4OOMU4QA.

Geoscience Australia Disclaimer: This e-mail (and files transmitted with it) is intended only for the person or entity to which it is addressed. If you are not the intended recipient, then you have received this e-mail by mistake and any use, dissemination, forwarding, printing or copying of this e-mail and its file attachments is prohibited. The security of emails transmitted cannot be guaranteed; by forwarding or replying to this email, you acknowledge and accept these risks.

dunkgray commented 4 years ago

Cool, looks good. I've given this an approval, but it is failing with CI checks. After they are fixed either you or I can do the merge.