search

davidgasquez / gitcoin-grants-data-portal

🌲 Open source, serverless, and local-first data hub for Gitcoin Grants data!
https://grantsdataportal.xyz/
MIT License
26 stars 3 forks source link

Interrupt CI run if `make run` fails #26

Closed DistributedDoge closed 7 months ago

DistributedDoge commented 8 months ago

After merge of #22 multiple tables silently failed to materialize breaking the portal as shown in run logs which contain RUN_FAILURE event during make run step.

dagster._core.errors.DagsterExecutionStepExecutionError: Error occurred while executing op "raw_passport_scores"::
ValueError: Unexpected character found when decoding array value (2)

Subsequent styling commit re-triggered the CI building all the tables. Since error had occured in table passport_scores which wasn't touched by either of the commits it is possible it was caused by something going on with the datasource.

To prevent such silent regressions, we could use exit status of make run so that github CI run is interrupted before IPFS upload occurs. Problem is, that dagster CLI always returns 0 exit code, even if run concludes with RUN_FAILURE event in the logs.

dagster asset materialize --select * -m ggdp
echo $?

Best solution I can think of at the moment is searching logs of make run with grep and throwing exit 1 if RUN_FAILURE is encountered.

Opening this one to brainstorm in case there was some less-janky way of coercing dagster asset materialize command to signal that something went wrong during the process.

davidgasquez commented 8 months ago

Thanks for surfacing this one! It's been on my mind to reach out to Dagster folks about this issue for a while... and this was the final ping :sweat_smile:

https://github.com/dagster-io/dagster/issues/19009

Lets see if they get back to us!

davidgasquez commented 8 months ago

Woah! They just "fixed" it. Will be available in the next Dagster release.

davidgasquez commented 7 months ago

I think today is release day for Dagster.

we don't have to do any changes on our own because... I haven't pinned any dependencies. I like living in the bleeding edge but wanted to raise that. As the project matures, we'll need to pin some things, specially core stuff like Dagster that also changes a lot! :sweat_smile:

DistributedDoge commented 7 months ago

image

New version hit release 4 hours ago, made a test run on branch with assets modified to fail to see what happens.

Works exactly as we wanted it to => failing to construct any asset (or pass any test) will lead to entire CI job run being interrupted at the end of make run step.