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

main.yml #316

Merged
merged 15 commits into from
May 4, 2022
Merged

main.yml #316

merged 15 commits into from
May 4, 2022

Conversation

Abdulrahim2567
Copy link
Contributor

Please check out the Ocaml 5.0.0+trunk+serial yml file I tried to move from the Drone CI to GitHub Actions.

@Sudha247 Sudha247 mentioned this pull request Mar 31, 2022
Copy link
Contributor

@Sudha247 Sudha247 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is neat! The code looks correct to me, although I'm not sure why the GitHub Actions job isn't triggered with the PR.

I'd rename the file to something more generic like main.yml since other jobs in the travis workflow need to be also included in this. Are you able to do the same for the other jobs too?

@Abdulrahim2567
Copy link
Contributor Author

Abdulrahim2567 commented Mar 31, 2022

Thanks, this is neat! The code looks correct to me, although I'm not sure why the GitHub Actions job isn't triggered with the PR.

I'd rename the file to something more generic like main.yml since other jobs in the travis workflow need to be also included in this. Are you able to do the same for the other jobs too?

I will work on the other jobs and commit them ASAP.

renamed OCAML-5.0.0+trunk+serial.yml to main.yml
removed [ continue-on-error: true ] so as to be informed if a job fails.
@Abdulrahim2567 Abdulrahim2567 changed the title OCAML-5.0.0+trunk+serial.yml main.yml Mar 31, 2022
@Abdulrahim2567
Copy link
Contributor Author

@Sudha247 @shakthimaan please review the latest I have made to this commit

add the other jobs from Drone CI to GitHub Actions
@Abdulrahim2567
Copy link
Contributor Author

Abdulrahim2567 commented Mar 31, 2022

@Sudha247 @shakthimaan An attempt to fix #275 . I have added the other jobs from Drone CI to GitHub actions please take a look at them :)

@Sudha247
Copy link
Contributor

Thanks @Abdulrahim2567! I'll try to activate this run on a local branch and get back to you.

@Abdulrahim2567
Copy link
Contributor Author

Thanks for having a look at the pull request. I will be waiting for your feedback. @Sudha247

.github/workflows/main.yml Outdated Show resolved Hide resolved
Relaxed on [push] constraint to trigger the workflow on all branches
@Abdulrahim2567
Copy link
Contributor Author

@Sudha247 just relaxed the on [push] constraint for the workflow to make it run on all branch branches, Could you please take a look at it? :)

@Abdulrahim2567
Copy link
Contributor Author

@Sudha247 I also tested it on my branch and it looks great. There is a new pull request coming from my branch starting with Abdulrahim2567

@shakthimaan
Copy link
Contributor

There is a new pull request coming from my branch

Please do not create multiple pull requests for the same issue. We will lose the conversation and the context. You can choose to rebase the commits to a single commit in the PR.

Added a newline between each job for clarity and readability purposes
@Abdulrahim2567
Copy link
Contributor Author

@shakthimaan Hello sir, I already closed the pull request just so we don't lose context of this PR :)

.github/workflows/main.yml Outdated Show resolved Hide resolved
Setting workflow to use ocaml-setup
@Abdulrahim2567
Copy link
Contributor Author

Abdulrahim2567 commented Apr 6, 2022

@shakthimaan @Sudha247 Hi, I just looked up this workflow run and I got an unexpected error, what could be wrong?. Please take a look:
Screenshot 2022-04-07 001234

@Abdulrahim2567
Copy link
Contributor Author

runs passed so far:-
runs

@Abdulrahim2567
Copy link
Contributor Author

Abdulrahim2567 commented Apr 6, 2022

Been trying to figure out what went wrong for hours now, any pointers to a possible solution?

.github/workflows/main.yml Outdated Show resolved Hide resolved
Removed every instance of "sudo chown -R opam" from main.yml as it is not supported on GitHub Actions.
@Abdulrahim2567
Copy link
Contributor Author

@Sudha247 @shakthimaan I just updated this workflow and removed every instance of sudo chown -R opam . but still got an error on this script USE_SYS_DUNE_HACK=1 RUN_CONFIG_JSON=run_config_filtered.json make ocaml-versions/5.0.0+stable.bench. Here is a the visual.

Screenshot 2022-04-07 153246

Any idea on what could've gone wrong?

@Sudha247
Copy link
Contributor

Sudha247 commented Apr 8, 2022

Discussed this with @shakthimaan offline, I suspect the run might be failing due to the CPU number 5 specified here.

GitHub Actions runners typically have only 2 cores, making only 0 and 1 valid CPU numbers. Drone seems to fallback on the available CPUs, but GitHub Actions doesn't. We need to find a viable way to change this in the CI.

@Abdulrahim2567
Copy link
Contributor Author

Abdulrahim2567 commented Apr 8, 2022

@Sudha247 Ahh I see. What do you think we should do now. Is there anyway which can us to solve this issue?
Or anything possible I can do for this whilst we find a possible solution?

@shakthimaan
Copy link
Contributor

failing due to the CPU number 5

For each of the entries in the main.yml file, before the TAG='"run_in_ci"'... step is invoked, add a sed command to replace the --cpu-list 5 to --cpu-list 1 for the respective .json file being used.

Added a `sed` command to replace `--cpu-list 5` to `--cpu-list 1` in json file the workflow uses.
Hi, Just added the `sed` to replace the `--cpu-list 5` to `--cpu-list 1`
@Abdulrahim2567
Copy link
Contributor Author

Abdulrahim2567 commented Apr 8, 2022

@shakthimaan @Sudha247 I just added a sed command to replace --cpu-list 5 to --cpu-list 1 for each .json file used by each job but got the following result for 5.0.0+stable. Did I miss something?
I also tried running sed for the run_config.json but then I got the same results which led me to using sed

Screenshot 2022-04-08 231922

Copy link
Contributor

@Sudha247 Sudha247 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sed command to replace CPU number is correct, but it's called before the generating the config file. Some suggestions are below to fix it. The same applies for all the sed commands used here.

.github/workflows/main.yml Outdated Show resolved Hide resolved
Abdulrahim2567 and others added 2 commits April 11, 2022 09:27
Modified the `sed` command to edit `run_config_filtered.json` by replacing `--cpu-list 5` by `--cpu-list 1`

Co-authored-by: Sudha Parimala <sudharg247@gmail.com>
Modified the `sed` commands to edit the `.json` files used by all jobs by changing all `--cpu-list 5` to `--cpu-list 1`
@Abdulrahim2567
Copy link
Contributor Author

Abdulrahim2567 commented Apr 11, 2022

@Sudha247 @shakthimaan Hello, I monitored the run which was triggered after modifying all sed commands from the workflow and got this issue to deal with.
ImportError: cannot import name 'soft_unicode' from 'markupsafe' (/home/runner/.local/lib/python3.8/site-packages/markupsafe/__init__.py)
coming from test_notebooks, any pointers to a possible fix?

Screenshot 2022-04-11 101142

The runs Passed so far

testpassed

@shakthimaan
Copy link
Contributor

I monitored the run which was triggered after modifying all sed commands from the workflow

In future, please test the changes locally on your system to see if it works, before pushing an update to the PR.

ImportError: cannot import name 'soft_unicode' from 'markupsafe'

This could be related, pallets/markupsafe#284.

@Abdulrahim2567
Copy link
Contributor Author

@shakthimaan okay no problem sir, I will keep that in mind before any future PR

Installed `markupsafe 2.0.1` instead of `markupsafe 2.1.1`
@Abdulrahim2567
Copy link
Contributor Author

@Sudha247 @shakthimaan I just updated the workflow to use markupsafe 2.0.1 and it worked out well please check it out

Abdulrahim2567 and others added 2 commits April 20, 2022 05:51
added `OPT_WAIT=0` to avoid load average checks for the CI runs
@shakthimaan shakthimaan merged commit cc20d4e into ocaml-bench:main May 4, 2022
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.

3 participants