Add CML Action to run whole workflow

radistoubalidis commented 2 years ago

Description

Goals

[x] Update action to run the whole simulation
[x] Add GCBM.CompileResults tool to the docker image
[ ] Compare numerical results and log files with HEAD

Type of change

[ ] New feature (non-breaking change which adds functionality)
[ ] This change requires a documentation update

How Has This Been Tested?

...

Additional Context (Please include any Screenshots/gifs if relevant)

Using the script written by Namyalg to retrieve the log files

HarshCasper commented 2 years ago

The CI has failed:

-rw-r--r-- 1 runner docker 1482752 Jun 24 07:01 gcbm_input.db
curl: (26) Failed to open/read local data from file/application
Error: Process completed with exit code 26.

Can you please investigate?

radistoubalidis commented 2 years ago

The CI has failed:

-rw-r--r-- 1 runner docker 1482752 Jun 24 07:01 gcbm_input.db
curl: (26) Failed to open/read local data from file/application
Error: Process completed with exit code 26.

Can you please investigate?

That error occured because I haven't pulled the changes from master, I upated it but now workflow is stuck at /dynamic and getting this log locally

<2022-06-24 09:27:51.998630> (info) - Config has files: 7
<2022-06-24 09:27:51.999017> (info) - Using Logging Configuration: /app/input/run4/logging.conf
<2022-06-24 09:27:51.999066> (info) - Using configurations: 
<2022-06-24 09:27:51.999079> (info) - localdomain.json
<2022-06-24 09:27:51.999091> (info) - pools_cbm.json
<2022-06-24 09:27:51.999136> (info) - modules_cbm.json
<2022-06-24 09:27:51.999184> (info) - modules_output.json
<2022-06-24 09:27:51.999195> (info) - spinup.json
<2022-06-24 09:27:51.999202> (info) - variables.json
<2022-06-24 09:27:51.999209> (info) - internal_variables.json
<2022-06-24 09:27:51.999216> (info) - Using provider configurations: 
<2022-06-24 09:27:51.999223> (info) - provider_config.json
<2022-06-24 09:27:52.029048> (info) - Using operation manager: 
<2022-06-24 09:27:52.029184> (info) - spinup ON, sequencer - CBMSpinupSequencer
<2022-06-24 09:27:52.046274> (info) - GCBM version: 1.0.0
<2022-06-24 09:27:52.063944> (fatal) - main(306) - Value too large.

And the CI fails on /upload with status 500

aornugent commented 2 years ago

Ah ha! @radistoubalidis - this seems to be a permissions error: https://github.com/iterative/cml/issues/183

I think we have two options:

investigate the use of Github Environments to actions from external PRs to run
merge it and test manually, assuming it'll work once integrated with the repository.

radistoubalidis commented 2 years ago

Ah ha! @radistoubalidis - this seems to be a permissions error: iterative/cml#183

I think we have two options:
* investigate the use of Github Environments to actions from external PRs to run

* merge it and test manually, assuming it'll work once integrated with the repository.

@HarshCasper told me that forks don't have access to github.access_token , that's why the cml-action was working only in my fork.

Update - Regarding github environments we could:

Set up an environment by adding up to 6 reviewers and configure a rule where any job will run when at least one of the reviewers approved
Set up some secrets that could be accessed only if the above rule has passed.

aornugent commented 2 years ago

Hi @radistoubalidis I missed your reply sorry.

Your proposed solutions sound great. However given that this is demonstrated to work on your fork, I think it will be ok to merge and review manually.

@HarshCasper WDYT? We could file an issue to configure Github environments in the future, noting that we cannot review PRs that require GHA secrets. I don't think we will have many of these.

HarshCasper commented 2 years ago

Hi @aornugent — I am merging this for now. I have reverted some of the changes that @radistoubalidis made, following you PR review, since it inadvertently broke the entire functionality which was too difficult to fix. I will create a separate issue to track the changes needed and maybe a first-time contributor can pick them up later. I hope that's fine.

radistoubalidis commented 2 years ago

In 03832a3 :

I modified CML Action to run the GCBM simulation without using the rest_gcbm_api .
- Until the api endpoints are refactored and merged from https://github.com/moja-global/FLINT.Cloud/pull/163 ,I'm using the linux-demo.zip configuration @aornugent uploaded in https://github.com/moja-global/FLINT.Cloud/pull/165 and interact directly with moja.cli using docker exec to run the simulation.
The simulation runs smoothly the job fails in the cml step because forks can't publish comments in a PR.
You can see full action output here

aornugent commented 2 years ago

PS please make sure to squash merge 😅

moja-global / FLINT.Cloud