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 cloud workflow for exporting vector data from Bigtable to Vertex Vector Search #930

Closed
wants to merge 9 commits into from

Conversation

ron-gal
Copy link

@ron-gal ron-gal commented Feb 5, 2024

…x Vector Search

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

This is a Bigtable version of https://github.com/cloudspannerecosystem/spanner-ai. Most of the work was copy pasting the Spanner code and adjusting it for Bigtable.

Fixes #<issue_number_goes_here> 🦕

@ron-gal ron-gal requested review from a team as code owners February 5, 2024 16:13
@ron-gal ron-gal requested a review from engelke February 5, 2024 16:13
Copy link

conventional-commit-lint-gcf bot commented Feb 5, 2024

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. api: bigtable Issues related to the googleapis/python-bigtable API. labels Feb 5, 2024
@ron-gal ron-gal changed the title Added cloud workflow for exporting vector data from Bigtable to Verte… feat: add cloud workflow for exporting vector data from Bigtable to Verte… Feb 5, 2024
@engelke engelke removed their request for review February 5, 2024 20:07
@daniel-sanche daniel-sanche changed the title feat: add cloud workflow for exporting vector data from Bigtable to Verte… feat: add cloud workflow for exporting vector data from Bigtable to Vertex Vector Search Feb 7, 2024
)

# Make the request
operation = workflow_client.delete_workflow(request=request)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put this in a try/finally block? I want to make sure it is being cleaned up properly, even if it exits early

Copy link
Author

Choose a reason for hiding this comment

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

Done

}
)

logger.info("Created {} table on instance {}.".format(table_name, instance_id))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the table being cleaned up properly on completion?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, everywhere this function is called it is followed by cleanup_bigtable_resources. Added try\finally in the one place it was missing, to ensure it happens.

@ron-gal ron-gal requested a review from daniel-sanche February 8, 2024 19:19
@daniel-sanche
Copy link
Contributor

I'm not sure this is the right place for this kind of content. Usually, the samples/ directory would just contain code snippets surrounded by region tags, that are then pulled into user-facing content on the docsite and/or in the library documentation in the docs/ folder. I'm not really seeing any region tags in this PR, and it seems more like a full tutorial. Are you sure this is the right place for it? Have you discussed this with the tech writers?

I'd probably suggest moving the contents of this README to a page on the docsite, pulling in some snippets like this one to show how to interact with it using Python, and linking to that page in the README here. But let me know if you've already considered that

@ron-gal
Copy link
Author

ron-gal commented Feb 12, 2024

Merged in GoogleCloudPlatform/workflows-demos#114

@ron-gal ron-gal closed this Feb 12, 2024
@ron-gal ron-gal deleted the bigtable_vertexme_workflow branch February 12, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the googleapis/python-bigtable API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants