Skip to content

Conversation

@huracosunah
Copy link
Contributor

Pull Request

NautilusTrader prioritizes correctness and reliability, please follow existing patterns for validation and testing.

  • I have reviewed the CONTRIBUTING.md and followed the established practices

Summary

Related Issues/PRs

Type of change

  • Bug fix (non-breaking)
  • New feature (non-breaking)
  • Breaking change (impacts existing behavior)
  • Documentation update
  • Maintenance / chore

Breaking change details (if applicable)

Documentation

  • Documentation changes follow the style guide (docs/developer_guide/docs.md)

Release notes

  • I added a concise entry to RELEASES.md that follows the existing conventions (when applicable)

Testing

Ensure new or changed logic is covered by tests.

  • Affected code paths are already covered by the test suite
  • I added/updated tests to cover new or changed logic

@CLAassistant
Copy link

CLAassistant commented Oct 19, 2025

CLA assistant check
All committers have signed the CLA.

@faysou
Copy link
Collaborator

faysou commented Oct 19, 2025

Looks fine to me

Removed 'region' option from S3 storage configurations.
@huracosunah
Copy link
Contributor Author

Also, I could get abfs Azure to work, but I had to explicitly handle abfs:// separately from azure://. The az:// protocol (recommended) is not supported in the rust session.rs or parquet.py.

For example:
elif self.fs_protocol in ("abfs"):
file_uri = f"{self.path}/{file.partition('/')[2]}"

So that the container isn't repeated in the uri. Where path abfs://[email protected]

Supporting all three protocols is more work, but I haven't built the rust layer yet to test.

Should the Azure fixes go into this PR or a separate one? What do you think?

@faysou
Copy link
Collaborator

faysou commented Oct 19, 2025

Better in a separate one

@cjdsellers
Copy link
Member

Hi @huracosunah

Thanks for the contribution!

Looks like the pre-commit failed, but this is easy to fix. Just run the following and push any changes to the same PR:

make pre-commit

or (if you're on Windows):

pre-commit run --all-files

Also consider running the following to install the git hook which will run the pre-commit automatically on commit:

pre-commit install

@cjdsellers cjdsellers changed the title Add filesystem parameter to parquet in the consolidate functions. Add filesystem parameter to parquet in the consolidate functions Oct 20, 2025
@cjdsellers cjdsellers merged commit 5da608d into nautechsystems:develop Oct 20, 2025
18 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.

4 participants