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

[eas-build] Expose generic job's BuildWorkflow #444

Conversation

sjchmiela
Copy link
Contributor

@sjchmiela sjchmiela commented Sep 26, 2024

Why

Exposing BuildWorkflow from runGenericJobAsync lets us fetch individual steps and their outputs after the job is complete.

How

  • wraps buildWorkflow.executeAsync() with asyncResult and returns the result from runGenericJobAsync alongside buildWorkflow,
  • adds outputs to Generic.Job so the user has a way of telling us which steps outputs are to be considered jobs outputs,
  • when a generic job finishes, Worker sends a robotAccessToken-authenticated request to a new www endpoint.

Also changes the way we're handling outputsDir and envsDir — we want the outputs and environment variables to be collected always, not only if the step succeeded. And exposes jsepEval so we can use it in Worker.

Moreover, removes outputs from BuildStep. We had two: outputs and outputById, one being updated as we process the workflow and the other not.

Test Plan

Adjusted tests. I've used this version to run a custom job with set-output name test and gotten

Zrzut ekranu 2024-09-27 o 18 44 42

Copy link

linear bot commented Sep 26, 2024

@sjchmiela sjchmiela force-pushed the stanley/eng-13458-add-support-for-job-outputs-other-than-entity-ids branch from fe87540 to a8dfd2e Compare September 26, 2024 22:13
@sjchmiela sjchmiela force-pushed the stanley/eng-13458-add-support-for-job-outputs-other-than-entity-ids branch from e45417e to 80c3fd0 Compare September 27, 2024 16:09
@sjchmiela sjchmiela marked this pull request as ready for review September 27, 2024 16:44
@sjchmiela sjchmiela force-pushed the stanley/eng-13458-add-support-for-job-outputs-other-than-entity-ids branch from 80c3fd0 to d666b49 Compare September 27, 2024 21:10
Copy link
Contributor

@radoslawkrzemien radoslawkrzemien left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

What's the rationale behind returning runResult: Result<void>;?

Copy link
Member

@szdziedzic szdziedzic left a comment

Choose a reason for hiding this comment

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

looks reasonable

@sjchmiela
Copy link
Contributor Author

What's the rationale behind returning runResult: Result<void>;?

Most importantly, I wanted to be able to return output always, if the job succeeds or fails. This is how it works in GitHub Actions too. Not sure if people use it, but if we can do it too… why not!

I could do { buildWorkflow, error: Error | null } to denote whether the workflow succeeded or failed, or something with type too. In the end I figured we can use Expo's own results library.

@sjchmiela sjchmiela merged commit 9429b17 into main Sep 30, 2024
5 checks passed
@sjchmiela sjchmiela deleted the stanley/eng-13458-add-support-for-job-outputs-other-than-entity-ids branch September 30, 2024 17:21
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