Skip to content

refactor(ais): Add backward compatibility and improve error handling in AISBatchLoader#1542

Merged
pzelasko merged 1 commit intolhotse-speech:masterfrom
gaikwadabhishek:get-batch-fallback
Feb 20, 2026
Merged

refactor(ais): Add backward compatibility and improve error handling in AISBatchLoader#1542
pzelasko merged 1 commit intolhotse-speech:masterfrom
gaikwadabhishek:get-batch-fallback

Conversation

@gaikwadabhishek
Copy link
Contributor

@gaikwadabhishek gaikwadabhishek commented Jan 13, 2026

  • Add backward compatibility for older AIStore SDK versions (Colocation, ArchiveConfig)
  • Move all aistore imports to method level (remove top-level imports)
  • Replace module-level logging with logger instance for better configuration
  • Fix return type annotation for _collect_manifest_urls (None -> bool)
  • Add safe attribute access with getattr() in error messages
  • Simplify ValueError handling with early return

Tests:

  • Add version compatibility tests for Colocation and ArchiveConfig fallbacks

This improves SDK version compatibility and code robustness while maintaining
full backward compatibility with older AIStore deployments.

Signed-off-by: Abhishek Gaikwad gaikwadabhishek1997@gmail.com

@gaikwadabhishek gaikwadabhishek marked this pull request as draft January 13, 2026 18:56
@gaikwadabhishek gaikwadabhishek changed the title Add error handling and fallback mechanism in AISBatchLoader refactor(ais): Add backward compatibility and improve error handling in AISBatchLoader Feb 11, 2026
@gaikwadabhishek gaikwadabhishek marked this pull request as ready for review February 11, 2026 20:14
pzelasko
pzelasko previously approved these changes Feb 13, 2026
Copy link
Collaborator

@pzelasko pzelasko left a comment

Choose a reason for hiding this comment

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

Looks good to me, but maybe it would be easier to just require an updated version of AIStore to simplify the logic here? Can you add to setup.py in a new item extras_require={..., "aistore": "aistore>=min-ver"} ?

…in AISBatchLoader

- Add backward compatibility for older AIStore SDK versions (Colocation, ArchiveConfig)
- Move all aistore imports to method level (remove top-level imports)
- Replace module-level logging with logger instance for better configuration
- Fix return type annotation for _collect_manifest_urls (None -> bool)
- Add safe attribute access with getattr() in error messages
- Simplify ValueError handling with early return

Tests:
- Add version compatibility tests for Colocation fallback

This improves SDK version compatibility and code robustness while maintaining
full backward compatibility with older AIStore deployments.

Signed-off-by: Abhishek Gaikwad <gaikwadabhishek1997@gmail.com>
@gaikwadabhishek
Copy link
Contributor Author

added

f"returned empty content. Retrying with direct AIStore API call."
)
try:
content = self._get_object_from_moss_in(info)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why run individual GET every time content is empty? Can we read the returned status to determine if there was an issue, or it was OK (in which case we don't send GET again)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we call batch.get(), if there are any errors (e.g., missing files, timeouts, connection resets), we place those entries under the __404__/ directory. Since we’re running GetBatch with cont_on_err, those files are created as empty placeholders. I’m using this as a backup strategy to preserve positional alignment and handle failures gracefully.

Copy link
Collaborator

@pzelasko pzelasko left a comment

Choose a reason for hiding this comment

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

LGTM with one comment (can be addressed later)

@pzelasko pzelasko enabled auto-merge (squash) February 20, 2026 14:58
@pzelasko pzelasko disabled auto-merge February 20, 2026 14:59
@pzelasko pzelasko merged commit 0ee2ce1 into lhotse-speech:master Feb 20, 2026
8 checks passed
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