Validate post-processing settings #36
Conversation
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against b43231e |
| assert cpus in cpus_to_memory_limits, "gcp.postprocessing_environment.cpus must be 1, 2, 4 or 8" | ||
| min_memory, max_memory = cpus_to_memory_limits[cpus] | ||
| assert min_memory <= memory, ( | ||
| f"gcp.postprocessing_environment.memory_mib must be at least {min_memory} for {cpus} CPUs. " |
There was a problem hiding this comment.
provide them with the link to more info (...google.cloud.run_v2.types.ResourceRequirements) here and in the next assert?
There was a problem hiding this comment.
Sure - moved the relevant links here instead of in comments.
| @@ -45,7 +45,7 @@ gcp-job-environment-spec: | |||
| use_spot: bool(required=False) | |||
|
|
|||
| gcp-postprocessing_environment-spec: | |||
| cpus: int(min=1, max=224, required=False) | |||
| cpus: int(min=1, max=8, required=False) | |||
There was a problem hiding this comment.
Add a link to where these value came from (...google.cloud.run_v2.types.ResourceRequirements)?
There was a problem hiding this comment.
Added (but used the general Cloud Run docs instead of the API docs)
buildstockbatch/gcp/gcp.py
Outdated
| @@ -942,8 +970,8 @@ def start_combine_results_job_on_cloud(self, results_dir, do_timeseries=True): | |||
| image=self.repository_uri + ":" + self.job_identifier, | |||
| resources=run_v2.ResourceRequirements( | |||
| limits={ | |||
| "memory": f"{pp_env_cfg.get('memory_mib', 4096)}Mi", | |||
| "cpu": str(pp_env_cfg.get("cpus", 2)), | |||
| "memory": f"{pp_env_cfg.get('memory_mib', self.DEFAULT_PP_MEMORY_MIB)}Mi", | |||
There was a problem hiding this comment.
should these use GcpBatch rather than self (or why is it inconsistent with the other usage above)?
There was a problem hiding this comment.
Either way works for accessing class variables. It's GcpBatch above because that's in a static method so there is no self.
Finish validation of GCP settings.
Adds checks for the post-processing resource settings, similar to the AWS checks here but based on the requirements from GCP.