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

Add context to triggers/automations #5758

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Feb 14, 2025

The focus of this PR is to get the automation triggers from Prefect working again in the develop branch. This is done by ensuring that we send the now required InfrahubContext object in different formats depending on if it's a trigger setup within Prefect (then we send it as JSON payload from the event.payload['context'] data and convert it into a dictionary instead of sending it as a string. The alternative is when se just submit or execute a workflow and send the context object as is.

The InfrahubContext object has been added as a required object to the event Metadata field. There are a few things to note about this:

  • With this in place some of the existing data in the metadata object such as account and branch are redundant. I didn't want to change to much within a single PR but before 1.2 is released we should phase out the old fields
  • As the AccountSession is required for the InfrahubContext I've added that information where applicable (with some dummy data within the tests). However we also have it in git/integrator.py where we don't yet have access to the context object. As things weren't working right now with the triggers I added some dummy data in that specific location as well. There I think we need to start from the beginning and see where we can create a proper AccountSession object and send it from the originating task (the other option would be to make the AccountSession optional but I think we can use it pretty much everywhere)

Because we always want to send the InfrahubContext as part of the event payload I added a new .get_event_payload() method that should only be defined on the root InfrahubEvent object and then child classes can override the get_payload() method without having to worry about the requirement to include the "context" key.

I added a new test to infrahub_docker where we validate that the trigger for the computed attribute (for Jinja) is working. So now we at least won't run into the same situation again. Those tests should be expanded on upcoming PRs though to also run the same type of tests for the Python Transform based computed attributes as well as testing the computed attributes on different branches.

I also enabled a test that was previously disabled. I'm not sure why it was failing before or if it started to work after the recent changes to the InfrahubEvent GraphQL query.

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Feb 14, 2025
Copy link

codspeed-hq bot commented Feb 14, 2025

CodSpeed Performance Report

Merging #5758 will not alter performance

Comparing pog-add-context-to-triggers-IFC-1244 (dbf932b) with develop (81a514f)

Summary

✅ 11 untouched benchmarks

@ogenstad ogenstad force-pushed the pog-add-context-to-triggers-IFC-1244 branch 3 times, most recently from 097e606 to 8ffd786 Compare February 14, 2025 13:41
@ogenstad ogenstad marked this pull request as ready for review February 14, 2025 14:27
@ogenstad ogenstad requested a review from a team as a code owner February 14, 2025 14:27
@ogenstad ogenstad force-pushed the pog-add-context-to-triggers-IFC-1244 branch from 8ffd786 to dbf932b Compare February 14, 2025 15:45
@ogenstad ogenstad merged commit 927ff90 into develop Feb 17, 2025
35 checks passed
@ogenstad ogenstad deleted the pog-add-context-to-triggers-IFC-1244 branch February 17, 2025 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
group/backend Issue related to the backend (API Server, Git Agent)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants