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

feat: Add teardown-project command #3598

Closed
wants to merge 3 commits into from

Conversation

jmeisele
Copy link

What this PR does / why we need it:

This PR adds an additional CLI command teardown-project. The existing teardown command brings ALL infrastructure down declared in the feature registry, including the registry itself. There are many instances when projects are using a shared registry for discoverability on the UI and they need the ability to bring down the project level infrastructure without interfering with each other.

Which issue(s) this PR fixes:

Fixes #3591

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jmeisele
To complete the pull request process, please assign adchia after the PR has been reviewed.
You can assign the PR to them by writing /assign @adchia in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jmeisele jmeisele changed the title feat: added teardown-project command feat: Add teardown-project command Apr 13, 2023
@jmeisele
Copy link
Author

/assign @adchia

@sfc-gh-madkins
Copy link
Collaborator

/ok-to-test

Signed-off-by: jmeisele <jmeisele@yahoo.com>
Signed-off-by: jmeisele <jmeisele@yahoo.com>
Signed-off-by: jmeisele <jmeisele@yahoo.com>
@sfc-gh-madkins
Copy link
Collaborator

@jmeisele dont we need to add logic that deletes all the registry data associated with the specific project as well?

@@ -1012,6 +1012,18 @@ def teardown(self):
self._get_provider().teardown_infra(self.project, tables, entities)
self._registry.teardown()

@log_exceptions_and_usage
def teardown_project(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems wrong since the registry file or sql registry itself still holds the references to the project. Probably better is to have each registry store also support tearing down references specific to a project?

Copy link
Author

Choose a reason for hiding this comment

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

💯 it's not as simple as first expected since the registries don't have parity in their behavior. I changed the PR to draft to chew on it a big longer and think through how each type of registry would solve this pattern. Thanks for the feedback @adchia 🙌🏻

@jmeisele jmeisele closed this Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants