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

Refactor TableClient to use Azure Data Tables SDK #278

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

AsafMah
Copy link
Contributor

@AsafMah AsafMah commented Jan 5, 2025

  • Replace deprecated storage package with aztables for table client operations.
  • Update NewTableClient to initialize an aztables.Client using the new SDK.
  • Modify Read and Write methods to utilize aztables API.
  • Add support for context propagation in Read and Write methods.
  • Serialize and deserialize entity data with JSON for compatibility with aztables.
  • Remove outdated table service and entity handling logic.

Added

Changed

Fixed

Removed

Security

- Replace deprecated `storage` package with `aztables` for table client operations.
- Update `NewTableClient` to initialize an `aztables.Client` using the new SDK.
- Modify `Read` and `Write` methods to utilize `aztables` API.
- Add support for context propagation in `Read` and `Write` methods.
- Serialize and deserialize entity data with JSON for compatibility with `aztables`.
- Remove outdated table service and entity handling logic.

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 7 changed files in this pull request and generated no comments.

Files not reviewed (3)
  • azkustodata/go.mod: Language not supported
  • azkustoingest/go.mod: Language not supported
  • go.work.sum: Language not supported
Comments suppressed due to low confidence (2)

azkustoingest/internal/status/status_table_client.go:62

  • The comment is incorrect. It should say 'Write writes a table record containing ingestion status.'
// Write reads a table record cotaining ingestion status.

azkustoingest/result.go:70

  • Using context.Background() here might not be ideal. Consider passing the context from the caller to ensure proper context propagation.
err = client.Write(context.Background(), r.record.IngestionSourceID.String(), r.record.ToMap())
Copy link

github-actions bot commented Jan 5, 2025

Test Results

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ❌ ±0 

Results for commit ff97a7f. ± Comparison against base commit 5a0ce31.

♻️ This comment has been updated with latest results.

@AsafMah AsafMah requested review from Copilot and yogilad January 7, 2025 13:52
@AsafMah AsafMah marked this pull request as ready for review January 7, 2025 13:52
Copy link
Contributor

@yogilad yogilad left a comment

Choose a reason for hiding this comment

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

Some comments, but looks ok

azkustoingest/internal/status/status_table_client.go Outdated Show resolved Hide resolved
azkustoingest/internal/status/status_table_client.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants