[Nebius] Nebius Object Storage support. #4838
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit introduces support for Nebius object storage, enabling users to integrate Nebius storage with various functionalities such as file mounting, syncing, and cloud transfers. It includes necessary adaptors, utility methods, and updates to the storage framework to handle Nebius-specific configurations and endpoints.
Implemented new Nebius storage mounting functionality, including COPY and MOUNT modes, with tests. Updated cloud_stores.py for AWS CLI compatibility and added a YAML template for Nebius storage configurations. Removed outdated Nebius test case in favor of the new approach.
This commit introduces comprehensive Nebius support, making it accessible for S3-compatible operations including bucket creation, deletion, and mounting. It removes reliance on the AWS_SHARED_CREDENTIALS_FILE environment variable, streamlines Nebius-specific configurations, and adds associated unit test parameters to validate functionality across storage operations.
cblmemo
reviewed
Mar 3, 2025
There was a problem hiding this comment.
Thanks for adding this @SalikovAlex ! This is exciting. Left some discussions ;)
Remove redundant code, streamline imports, and enhance error messaging. Adjust documentation for better accuracy and update function annotations. These changes improve maintainability and readability of the Nebius adaptor module.
Simplified Nebius AWS CLI installation by reusing the S3CloudStorage configuration for consistency. Removed unnecessary debug print in `run_upload_cli` to reduce console noise. Minor formatting adjustment in test YAML file.
Clean up and improve code readability, including string formatting and conditionals. Introduce `_TIMEOUT_TO_PROPAGATES` to handle timeout while verifying Nebius bucket deletions. Update comments to reflect the corrected usage on Nebius servers.
Removed unused variable from subprocess call to clean up code. Updated timeout error to include the bucket name for more detailed and helpful error reporting.
Updated the helper method to assign a default region when no region is specified, ensuring compatibility with Nebius Object Storage. This change avoids potential errors caused by missing region values.
Replace 'nebius://' with 's3://' in source paths to ensure compatibility with AWS CLI commands. This allows seamless integration of Nebius storage endpoints.
cblmemo
reviewed
Mar 11, 2025
There was a problem hiding this comment.
Thanks for the update @SalikovAlex ! The PR mostly looks good to me. Left some discussions - after that it should be ready to go!
Could you also help run the related smoke test on nebius?
sky/cloud_stores.py
Outdated
| # To increase parallelism, modify max_concurrent_requests in your | ||
| # aws config file (Default path: ~/.aws/config). | ||
| endpoint_url = nebius.create_endpoint() | ||
| if 'nebius://' in source: |
There was a problem hiding this comment.
If it starts with s3://, isn't it be treated as S3 storage and route to the S3Store class?
Updated the `create_endpoint` function to ensure the `region` parameter is strictly typed as `str`. Modified `create_nebius_client` to accept `None` as the default region. Additionally, corrected the error message in `storage.py` to specify 'nebius' instead of 's3'.
cblmemo
reviewed
Mar 12, 2025
Updated R2 command to explicitly set AWS_SHARED_CREDENTIALS_FILE for better credential management. Simplified region assignment logic in storage initialization to improve code readability and maintainability.
|
Looks like you are right. |
cblmemo
reviewed
Mar 13, 2025
There was a problem hiding this comment.
Thanks @SalikovAlex for fixing this! Left final comments. After addressing those, could you help running the related smoke test on your side?
sky/cloud_stores.py
Outdated
| # To increase parallelism, modify max_concurrent_requests in your | ||
| # aws config file (Default path: ~/.aws/config). | ||
| endpoint_url = nebius.create_endpoint() | ||
| if 'nebius://' in source: |
There was a problem hiding this comment.
I think here we also assume it can only start with `nebius://`
Ensure Nebius paths are properly validated and transformed, replacing `if` checks with assertions. Fixed default region handling in `create_endpoint` and corrected variable naming in `split_nebius_path` for consistency. These changes enhance code reliability and maintainability.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tested (run the relevant ones):
bash format.shpytest tests/smoke_tests/test_mount_and_storage.py::test_nebius_storage_mounts --nebiuspytest tests/test_smoke.pypytest tests/smoke_tests/test_mount_and_storage.py::test_nebius_storage_mounts --nebius