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

On-Board check-catalyst workflow to use GitHub Large Runners #675

Merged
merged 23 commits into from
Apr 25, 2024

Conversation

rashidnhm
Copy link
Collaborator

@rashidnhm rashidnhm commented Apr 23, 2024

Context:
When high volume of jobs are being queued on CI. It is possible that it will take a while before a job is picked up. To get around this, we have provisioned an organization runner group of GitHub Large Runners. Since these are paid runners, we retain the ability to scale this up to a value suitable for us during periods of high usage.

Description of the Change:
A new label urgent has been introduced to this repo. Whenever a pull request has this label attached, all the jobs within check-catalyst.yml will swap over to use the new Large runner group described above.

This PR also bumps the version of actions/checkout and actions/cache being used. This was done to deal with the deprecation of node16 within github actions.

Benefits:
Lowered queue times for urgent CI workloads to get picked up.

Possible Drawbacks:
We can still see jobs being queued for a while if enough PRs have the urgent label at once. To get around this, only add it to PRs that are truly urgent. Also, we can increase the scaling factor of the group if needed.

Related GitHub Issues:
None. sc-61791

[sc-61184]: fix for cache issues

@rashidnhm rashidnhm added the urgent Mark a pull request as high priority label Apr 23, 2024
@rashidnhm rashidnhm changed the title Try Large runners On-Board check-catalyst workflow to use GitHub Large Runners Apr 24, 2024
@rashidnhm rashidnhm marked this pull request as ready for review April 24, 2024 15:22
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/changelog.md on your branch with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@rashidnhm rashidnhm requested a review from dime10 April 24, 2024 15:30
Copy link
Contributor

@dime10 dime10 left a comment

Choose a reason for hiding this comment

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

Thanks @rashidnhm, can we include the linux wheels in this?

@rashidnhm rashidnhm added the author:build-wheels Run the wheel building workflows on this Pull Request label Apr 24, 2024
@dime10
Copy link
Contributor

dime10 commented Apr 24, 2024

It would be nice to upgrade invoked actions across all our workflows (like what you did in check-catalyst.yml), or would you like to do that in a follow up?

@rashidnhm
Copy link
Collaborator Author

It would be nice to upgrade invoked actions across all our workflows (like what you did in check-catalyst.yml), or would you like to do that in a follow up?

It shouldn't be that much effort to upgrade I'll add it in within this pr

Copy link
Contributor

@erick-xanadu erick-xanadu left a comment

Choose a reason for hiding this comment

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

I think we need to revert the checkout action to version 3: actions/checkout#1487 actions/checkout#1474

@erick-xanadu
Copy link
Contributor

I think that we can switch to checkout action v4 when we switch to another container. But manylinux_2014 is likely unsupported. See these screenshots and this link:
tarantool/tarantool@686bd70

Screenshot from 2024-04-25 16-21-22
Screenshot from 2024-04-25 16-21-44

@rashidnhm rashidnhm merged commit dc9ad8a into main Apr 25, 2024
58 checks passed
@rashidnhm rashidnhm deleted the sc-61791-ci-use-large-runners branch April 25, 2024 22:46
erick-xanadu pushed a commit that referenced this pull request Apr 26, 2024
**Context:** A bug was introduced with #675 which caused Linux builds to
be skipped. This PR fixes the issue.

On the GitHub UI, it showed all checks as passing which caused the issue
to slip by.

When `fromJSON` attempts to parse a string that is invalid JSON, it
returns an empty string. This caused the strategy matrix to fail.

**Description of the Change:**
Fixed formatting of format function feeding into fromJSON.

**Benefits:**
Linux jobs won't fail due to invalid workflow definition.

**Possible Drawbacks:**
None.

**Related GitHub Issues:**
None.
[sc-61791](https://app.shortcut.com/xanaduai/story/61791/update-catalyst-ci-to-use-large-runners)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author:build-wheels Run the wheel building workflows on this Pull Request urgent Mark a pull request as high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants