Skip to content

Conversation

niteesh-atlan
Copy link

Changelog

path was always set to none as the underlying methods inside download_files is returning None

Additional context (e.g. screenshots, logs, links)

Screenshot 2025-09-23 at 10 35 39 AM Screenshot 2025-09-23 at 10 35 31 AM Screenshot 2025-09-23 at 10 35 14 AM

Checklist

  • Additional tests added
  • All CI checks passed
  • Relevant documentation updated

Copyleft License Compliance

  • Have you used any code that is subject to a Copyleft license (e.g., GPL, AGPL, LGPL)?
  • If yes, have you modified the code in the context of this project? please share additional details.

@Copilot Copilot AI review requested due to automatic review settings September 23, 2025 05:06
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where the download_files method in the parquet input handler was not returning the downloaded file path, causing the code to always use the original remote path instead of the local downloaded path.

  • Removes unnecessary local path variable assignment
  • Updates download_files call to not capture return value since it modifies self.path directly
  • Ensures pd.read_parquet() uses the correct path after download

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +93 to +95
await self.download_files(self.path)
# Use pandas native read_parquet which can handle both single files and directories
return pd.read_parquet(path)
return pd.read_parquet(self.path)
Copy link
Preview

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

The fix assumes that download_files modifies self.path in-place, but this behavior is not evident from the code shown. If download_files doesn't update self.path to point to the downloaded local file, this will still read from the original remote path. Consider either ensuring download_files updates self.path or capturing its return value if it returns the local path.

Copilot uses AI. Check for mistakes.

Comment on lines 92 to +95
if self.input_prefix and self.path:
path = await self.download_files(self.path)
await self.download_files(self.path)
# Use pandas native read_parquet which can handle both single files and directories
return pd.read_parquet(path)
return pd.read_parquet(self.path)
Copy link
Preview

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

The fix assumes that download_files modifies self.path in-place, but this behavior is not evident from the code shown. If download_files doesn't update self.path to point to the downloaded local file, this will still read from the original remote path. Consider either ensuring download_files updates self.path or capturing its return value if it returns the local path.

Copilot uses AI. Check for mistakes.

@inishchith inishchith added e2e-test run-examples Run examples on the Pull Request labels Sep 23, 2025
Copy link
Member

@inishchith inishchith left a comment

Choose a reason for hiding this comment

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

can you look at the conflicts, comments from copilot and add a quick test for this? thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e-test run-examples Run examples on the Pull Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants