Skip to content

chore!: Remove DataSourceTask.get_micro_partitions - #7611

Open
srilman wants to merge 1 commit into
slade/upgrade-final-depsfrom
slade/remove-get-micro-partitions
Open

srilman wants to merge 1 commit into
slade/upgrade-final-depsfrom
slade/remove-get-micro-partitions

Conversation

@srilman

@srilman srilman commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Changes Made

Remove deprecated DataSourceTask.get_micro_partitions. Users should use DataSourceTask.read

@srilman
srilman requested a review from a team as a code owner October 16, 2026 01:30
@github-actions

Copy link
Copy Markdown

Rust Dependency Diff

Head: 7c4ba4df2cd194c9ae44944bc73ad5f354955097 vs Base: 7268a0775602cd7e06283393934e2e0f072bb6ac.

✅ OK: Within budget.

  • New Crates: 5
  • Removed Crates: 7

Added

  • azure_storage_blob: 1.1.0
  • azure_storage_common: 1.0.0
  • regex: 1.13.1
  • regex-automata: 0.4.18
  • regex-syntax: 0.8.11

Removed

  • async-compat: 0.2.5
  • azure_storage_blob: 1.1.0-beta.1
  • azure_storage_common: 0.1.0
  • path_macro: 1.0.0
  • regex: 1.12.3
  • regex-automata: 0.4.14
  • regex-syntax: 0.8.10

@srilman
srilman changed the base branch from main to slade/upgrade-final-deps October 16, 2026 01:30
@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[High impact] Removes deprecated API from data source task interface.

The PR appears safe to merge, with a non-blocking request for a two-page Azure listing test.

Summary

Requires custom DataSourceTask implementations to provide async read instead of get_micro_partitions.

  • Python data tasks provide batches through read().
  • Azure directory listings use the standard blob-list request with a delimiter.
  • Supporting Rust dependencies and helpers are simplified without changing their behavior.

The removal of legacy-only custom tasks is intentional in the PR description supplied by srilman; callers must implement read.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Directory listing] --> B[Context carries delimiter]
    B --> C[ListBlobsDelimiterPolicy adds query parameter]
    C --> D[SharedKey signs request]
    D --> E[Azure returns a page]
    E --> F[Parse files and directories]
    E --> G[SDK follows NextMarker]
    G --> C
Loading

Reviews (1) · Last reviewed commit: "remove another deprecation" · Reviewed by Greptile

@greptile-apps

greptile-apps Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff

These findings could not be posted inline.

  • P2 Pagination test bypasses the client src/daft-io/src/azure_blob.rs:1125 ▶

    The new check reads XML into a test-only ListPage; it never exercises list_blobs().into_pages() or ListBlobsDelimiterPolicy. It would still pass if later requests lost delimiter, or if the real pager mishandled an empty NextMarker.

    Add a two-page test through the Azure client with a mock server. Check that both requests carry delimiter=/, both pages return directories, and an empty marker stops further requests.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@codecov

codecov Bot commented Oct 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 76.88%. Comparing base (c9866a0) to head (591c8c3).

Files with missing lines Patch % Lines
daft/io/source.py 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                     Coverage Diff                      @@
##           slade/upgrade-final-deps    #7611      +/-   ##
============================================================
- Coverage                     77.21%   76.88%   -0.34%     
============================================================
  Files                          1191     1191              
  Lines                        173683   173674       -9     
============================================================
- Hits                         134115   133522     -593     
- Misses                        39568    40152     +584     
Files with missing lines Coverage Δ
daft/file/file.py 88.48% <ø> (ø)
daft/io/source.py 94.33% <50.00%> (+2.40%) ⬆️

... and 21 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants