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: issues with ecs-task-runner during prototyping #22

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

therealvio
Copy link
Contributor

@therealvio therealvio commented Dec 17, 2024

Purpose 🎯

These changes feature various fixes that were identified during prototyping of the Buildkite plugin for the permissions-service. These fixes will ensure that the plugin will be suitable for future projects to use.

Overall the changes are as follows:

  1. kebab-casing is required for Buildkite plug-in properties. otherwise the expected environment variables that the plug-in produces will not be named as what is expected. the kebab-case translates into upper snake case eventually.
  2. Newlines have been added to ensure that logs are spaced out correctly and not presented as contiguous strings.
  3. A missing pointer dereference has been added that allows proper comparison of the exit code returned by tasks.

Context 🧠

@therealvio therealvio force-pushed the working-the-prototype branch from c71c2f0 to 7bcab5b Compare December 17, 2024 05:41
Prior to this change, the parameter name was in camelCase, and this
casing appears to transfer over when Buildkite sets up the environment
variables for the plugin.

This change uses kebab case for the parameter, which is the casing
Buildkite expects. In turn, this transforms in UPPER_SNAKE_CASE when
the environment variables are set up.
This change fixes a missing pointer dereference so that the exit code of
the task is correctly compared, and the correct message is returned
based on the comparison.
@therealvio therealvio force-pushed the working-the-prototype branch from 7bcab5b to 1ce353d Compare December 17, 2024 05:42
@therealvio therealvio marked this pull request as ready for review December 17, 2024 05:42
Although the primary use of the task runner is for database migrations,
there isn't anything specific to the code that indicates it is only
capable of doing this.

Hence, we will keep the name of the tasks and container generic, and
leave the application up to the people who consume the plugin.
@therealvio therealvio force-pushed the working-the-prototype branch from 88a11cc to 83441e1 Compare December 18, 2024 04:42
Copy link

github-actions bot commented Jan 4, 2025

Package Line Rate Health
github.com/cultureamp/ecs-task-runner-buildkite-plugin/aws 81%
github.com/cultureamp/ecs-task-runner-buildkite-plugin/buildkite 0%
github.com/cultureamp/ecs-task-runner-buildkite-plugin 0%
github.com/cultureamp/ecs-task-runner-buildkite-plugin/plugin 4%
Summary 36% (84 / 234)

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.

2 participants