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

[SDK-1683] Single workflow with matrix strategy #1840

Merged
merged 5 commits into from
Jan 10, 2024
Merged

Conversation

maratal
Copy link
Collaborator

@maratal maratal commented Nov 19, 2023

Closes #1311

@github-actions github-actions bot temporarily deployed to staging/pull/1840/features November 19, 2023 23:33 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1840/jazzydoc November 19, 2023 23:40 Inactive
@maratal maratal force-pushed the fix/1311-single-workflow branch from 7f70719 to 70a06b7 Compare November 19, 2023 23:42
@github-actions github-actions bot temporarily deployed to staging/pull/1840/features November 19, 2023 23:42 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1840/jazzydoc November 19, 2023 23:46 Inactive
@maratal maratal force-pushed the fix/1311-single-workflow branch from 70a06b7 to 38d31c1 Compare November 19, 2023 23:51
@github-actions github-actions bot temporarily deployed to staging/pull/1840/features November 19, 2023 23:51 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1840/jazzydoc November 19, 2023 23:55 Inactive
@maratal maratal changed the title Get back single workflow with matrix strategy + cleaned up step names. [SDK-1683] Single workflow with matrix strategy Nov 20, 2023
@maratal maratal force-pushed the fix/1311-single-workflow branch from 38d31c1 to 3e5606e Compare November 20, 2023 00:20
@github-actions github-actions bot temporarily deployed to staging/pull/1840/features November 20, 2023 00:20 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1840/jazzydoc November 20, 2023 00:25 Inactive
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian left a comment

Choose a reason for hiding this comment

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

Something that I noticed is that there is now no way to tell which OS an observability server upload corresponds to. Compare the following:

Notice that in the first one, clicking the "GitHub run ID" or "GitHub run attempt" link takes you to the iOS job. In the second, it takes you to an overview page that shows all 3 OSs.

There is probably some other piece of data that we need to get from the environment and include in the upload to the observability server to allow it to display a link to the correct job.

@maratal
Copy link
Collaborator Author

maratal commented Dec 10, 2023

Something that I noticed is that there is now no way to tell which OS an observability server upload corresponds to. Compare the following:

* [an upload from before these changes](https://test-observability.herokuapp.com/repos/ably/ably-cocoa/uploads/b12c25dc-c3ac-4912-a6cc-6106ed026267)

* [an upload from after these changes](https://test-observability.herokuapp.com/repos/ably/ably-cocoa/uploads/358cbc52-d669-4e6a-827d-d3ac7f46e685)

Notice that in the first one, clicking the "GitHub run ID" or "GitHub run attempt" link takes you to the iOS job. In the second, it takes you to an overview page that shows all 3 OSs.

There is probably some other piece of data that we need to get from the environment and include in the upload to the observability server to allow it to display a link to the correct job.

Turned out it's not trivial. GITHUB_JOB is actually an alphabetical id ("check" in our case), and to get numerical id one need to use a custom parser action - this for example. Source https://stackoverflow.com/questions/71240338/obtain-job-id-from-a-workflow-run-using-contexts

@lawrence-forooghian
Copy link
Collaborator

I think we need to do that then. Sounds like the steps are the following:

  1. before uploading to the observability server, fetch the jobs for the current workflow attempt
  2. find the currently-running job from the response's jobs array (I believe, according to what this says about the order of job creation, that the $n^{th}$ item there corresponds to the $n^{th}$ item in the matrix, so we can fetch it by using the index of the current job in the matrix, which we might need to pass as an additional variable from the matrix to the job)
  3. extract the html_url from that job and upload it as a new parameter (e.g. GITHUB_JOB_URL, I can add support to the server for storing and displaying this value) to the observability server

@lawrence-forooghian
Copy link
Collaborator

I've actually just needed to do the same thing for ably-js. Here's the PR for adding this functionality to the upload; it should help you here. ably/test-observability-action#20

@lawrence-forooghian
Copy link
Collaborator

I've implemented it for ably-cocoa in #1845. You'll just need to pass the --job-index argument in this PR (in your case it'll be ${{ strategy.job-index }}).

@maratal
Copy link
Collaborator Author

maratal commented Dec 14, 2023

I've implemented it for ably-cocoa in #1845. You'll just need to pass the --job-index argument in this PR (in your case it'll be ${{ strategy.job-index }}).

This is wonderful, thanks Lawrence!

# Conflicts:
#	.github/workflows/integration-test-iOS16_2.yaml
#	.github/workflows/integration-test-tvOS16_1.yaml
@github-actions github-actions bot temporarily deployed to staging/pull/1840/features December 14, 2023 23:11 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1840/jazzydoc December 14, 2023 23:15 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1840/features December 15, 2023 00:32 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1840/jazzydoc December 15, 2023 00:39 Inactive
@maratal maratal force-pushed the fix/1311-single-workflow branch from 989feaf to 491aa20 Compare December 15, 2023 01:02
@github-actions github-actions bot temporarily deployed to staging/pull/1840/features December 15, 2023 01:03 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1840/jazzydoc December 15, 2023 01:07 Inactive
@maratal maratal force-pushed the fix/1311-single-workflow branch from 491aa20 to 528342d Compare December 15, 2023 01:49
@github-actions github-actions bot temporarily deployed to staging/pull/1840/features December 15, 2023 01:49 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1840/jazzydoc December 15, 2023 01:52 Inactive
@maratal maratal force-pushed the fix/1311-single-workflow branch from 528342d to 6bd3ea8 Compare December 15, 2023 13:30
@github-actions github-actions bot temporarily deployed to staging/pull/1840/features December 15, 2023 13:30 Inactive
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian left a comment

Choose a reason for hiding this comment

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

Looks good — let's not merge this until ably/test-observability#76 is merged and deployed though.

@maratal maratal mentioned this pull request Jan 7, 2024
@lawrence-forooghian
Copy link
Collaborator

Looks good — let's not merge this until ably/test-observability#76 is merged and deployed though.

This is now deployed.

@lawrence-forooghian
Copy link
Collaborator

Re-running CI to check that the URL gets correctly populated in observability server. If it does, I'll approve.

@maratal maratal merged commit 29d4532 into main Jan 10, 2024
7 checks passed
@maratal maratal deleted the fix/1311-single-workflow branch January 10, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants