Update import status in spanner

[vish-cs]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @vish-cs, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the import automation system by introducing robust import status tracking capabilities using Google Cloud Spanner. The changes enable the system to record and update the progress of data imports in a centralized, highly available database, providing better visibility into the import workflow.

Highlights

• Spanner Integration: Introduced new configuration parameters for connecting to a Google Cloud Spanner instance, including project ID, instance ID, and database ID.
• Import Status Tracking: Implemented a new method _update_import_status_table to record and update the status of data imports in a Spanner database. This method inserts or updates entries in an 'ImportStatus' table with details like import name, latest version, state (initially 'PENDING'), job ID, and timestamp.
• Workflow Integration: Integrated the Spanner status update mechanism into the existing import workflow, specifically within the _update_latest_version function, ensuring that import status is logged to Spanner as new versions are processed.
• Dependency Update: Added google-cloud-spanner to the project's Python dependencies, enabling interaction with the Spanner service.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request adds functionality to update an import's status in a Spanner table. The changes include adding Spanner configuration, a new dependency, and a method to perform the database update. My review focuses on improving the implementation's robustness and performance. I've pointed out that the Spanner configuration is hardcoded, which is not ideal for different environments. I've also identified a performance issue with how the Spanner client is instantiated on every call, and a potential data integrity issue with how an environment variable is being read. The suggestions aim to make the code more resilient and efficient.

[gemini-code-assist]

The Spanner connection details are hardcoded and appear to point to a test environment (e.g., spanner_instance_id: str = 'dc-kg-test'). Hardcoding configuration like this makes it difficult to deploy and manage the application across different environments (development, staging, production). It's recommended to externalize this configuration, for instance, by reading these values from environment variables. This would allow you to configure the application for each environment without changing the code.

Suggested change

	spanner_project_id: str = 'datcom-store'
	spanner_instance_id: str = 'dc-kg-test'
	spanner_database_id: str = 'dc_graph_import'
	spanner_project_id: str = os.getenv('SPANNER_PROJECT_ID', 'datcom-store')
	spanner_instance_id: str = os.getenv('SPANNER_INSTANCE_ID', 'dc-kg-test')
	spanner_database_id: str = os.getenv('SPANNER_DATABASE_ID', 'dc_graph_import')

[gemini-code-assist]

A new Spanner client, instance, and database object are created on each call to this function. This is inefficient because these objects are expensive to initialize and are designed to be long-lived and reused.

It is highly recommended to refactor this to initialize the Spanner client and database objects once in the ImportExecutor constructor (__init__) and store them as instance attributes (e.g., self.spanner_database). Then, you can reuse self.spanner_database here.

Example of changes in __init__:

# In ImportExecutor.__init__
self.spanner_client = spanner.Client(
    project=self.config.spanner_project_id,
    client_options={'quota_project_id': self.config.spanner_project_id}
)
self.spanner_instance = self.spanner_client.instance(self.config.spanner_instance_id)
self.spanner_database = self.spanner_instance.database(self.config.spanner_database_id)

Then this method would simply use self.spanner_database.

[gemini-code-assist]

os.getenv('BATCH_JOB_UID') will return None if the environment variable is not set. This None value would then be inserted into the JobId column in Spanner, which might not be the desired behavior and could lead to data integrity issues. It's safer to handle this case explicitly by providing a default value if the job UID is not found.

Suggested change

	values=[(import_name, gcs_path, "PENDING",
	os.getenv('BATCH_JOB_UID'),
	spanner.COMMIT_TIMESTAMP)])
	values=[(import_name, gcs_path, "PENDING",
	os.getenv('BATCH_JOB_UID', 'unknown-job'),
	spanner.COMMIT_TIMESTAMP)])