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.

Sign up for GitHub

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

Jump to bottom

Remove unused files #9002

Open
wants to merge 17 commits into
base: staging
Choose a base branch
from
Open

Remove unused files #9002

wants to merge 17 commits into from

Conversation

ZoranPandovski
Copy link
Member

@ZoranPandovski ZoranPandovski commented Mar 27, 2024 •
edited

This PR removes unused:

  • protos directory
  • quickstart guide in docs
  • 1.35.0 file
  • empty api/nlp dir
  • integrations net_helpers
  • all files in microservices_grpc
  • grpc handlers wrappers
  • grpc db handlers clients
  • helm charts unmaintained directory

@ZoranPandovski ZoranPandovski marked this pull request as draft March 27, 2024 15:52
@ZoranPandovski ZoranPandovski marked this pull request as ready for review March 28, 2024 14:36
@ZoranPandovski
Copy link
Member Author

@StpMax , @ea-rus can you please double check this?

ea-rus
ea-rus reviewed Apr 1, 2024
View reviewed changes
mindsdb/interfaces/database/integrations.py
@@ -411,41 +399,6 @@ def _make_handler_args(self, name: str, handler_type: str, connection_data: dict

return handler_ars

def create_tmp_handler(self, handler_type: str, connection_data: dict) -> object:
Copy link
Contributor

Choose a reason for hiding this comment

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

this function is used, for example here

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only for the tests. Do we need this since it brings a lot of complexity with it, which is not required now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ZoranPandovski yes, 'test connection' does not work anymore
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

which is not required now?

@ZoranPandovski you mean not required in new cloud? but what about opensource version - should it have 'test connection' ability?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ea-rus ea-rus ea-rus left review comments

@StpMax StpMax Awaiting requested review from StpMax

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
Tracking PRs
Status: to review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ZoranPandovski @StpMax @ea-rus