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

fix(integration-templates): add workflow and edit existing #2337

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

khaliqgant
Copy link
Member

Describe your changes

With relative imports the logic needs to be adjusted some. Also adds a workflow to upload all templates as a safeguard as when a template is made by an external contributor it doesn't get uploaded due to lack of permissions

Issue ticket number and link

Checklist before requesting a review (skip if just adding/editing APIs & templates)

  • I added tests, otherwise the reason is:
  • I added observability, otherwise the reason is:
  • I added analytics, otherwise the reason is:

unset parts[-2]
fi

dir=$(IFS=/; echo "${parts[*]}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you want more bash magic I think you can do this with a one liner:

dir=${dir%/*/*}/${dir##*/}

Removing the last 2 elements of the path with the first curly bracket and re-adding the last element in the second

@@ -0,0 +1,39 @@
name: 'Sync Integration Templates'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure I follow why we need this extra action. Aren't the merge triggering the templates upload action above done by us, even on external contributions? if yes what's the permissions issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is more of a fail safe than anything. It’s a force sync of all templates since the regular one looks just for changed files.

For some reason this action contribution #2242 didnt get uploaded to s3. In that instance we need a way to sync integration templates. I need to look into exactly why it failed but I assumed it was a permissions issue but not 100% sure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion for future: this could be done at the start of the server process instead

@@ -0,0 +1,39 @@
name: 'Sync Integration Templates'
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion for future: this could be done at the start of the server process instead

@khaliqgant khaliqgant merged commit 16bca34 into master Jun 18, 2024
20 checks passed
@khaliqgant khaliqgant deleted the update-logic-for-integration-template-upload branch June 18, 2024 06:33
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