Skip to content
This repository has been archived by the owner on Nov 13, 2024. It is now read-only.

Cleanup indexes in case of failure #232

Merged
merged 18 commits into from
Dec 20, 2023

Conversation

izellevy
Copy link
Collaborator

@izellevy izellevy commented Dec 19, 2023

Problem

When an action get cancelled pytest is stopped using SIGKILL (by Github CI) which causes unused indexes to remain after the test.

Solution

Add a new step to clean up the indexes in case the run is interrupted in the middle.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

Describe specific steps for validating this change.

Copy link
Contributor

@igiloh-pinecone igiloh-pinecone left a comment

Choose a reason for hiding this comment

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

@izellevy see one comment, other than that LGTM

echo "Test Run ID: ${run_id}"
echo "run_id=${run_id}" >> $GITHUB_OUTPUT
- name: Cleanup indexes
if: failure() && github.event_name == 'merge_group'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if: failure() && github.event_name == 'merge_group'
if: always() && github.event_name == 'merge_group'

Let's not take any chances. The added time for successful tests is probably negligible.

@izellevy izellevy added this pull request to the merge queue Dec 20, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 20, 2023
Copy link
Contributor

@igiloh-pinecone igiloh-pinecone left a comment

Choose a reason for hiding this comment

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

LGTM!

logger.info("Usage: python scripts/cleanup_indexes.py <testrun_uid>")
sys.exit(1)

testrun_uid = sys.argv[1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please promise me that if we're every adding even more one parameter, we're switching to argparse 😄

@izellevy izellevy added this pull request to the merge queue Dec 20, 2023
Merged via the queue into pinecone-io:main with commit 9a3efc2 Dec 20, 2023
10 checks passed
@izellevy izellevy deleted the bug/cleanup_indexes branch December 20, 2023 15:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants