Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(files): add support for Files API (personal and shared spaces) #46

Merged

Conversation

nunogoncalves03
Copy link
Member

We have recently updated the Management API to include new endpoints for managing files in personal and shared spaces. Thus, we would like to update the Python SDK to support these resources as well.

Since the methods for stage files and personal/shared space files are quite similar, we would like to refactor the Python SDK by introducing an abstract class called FileLocation. This class will be implemented by Stage and FileSpace, which will be used to interact with the Files API for personal and shared files.

This PR introduces the FilesManager and FileSpace classes, allowing us to manage files in personal and shared spaces.

Note that this PR depends on #45 and only affects the Python SDK. To keep the PRs smaller, I will include the Fusion SQL changes related to the Files API in a separate PR.

cc: @ricardoasmarques @kanitsharma

@nunogoncalves03 nunogoncalves03 added the feature New feature or request label Nov 25, 2024
@nunogoncalves03 nunogoncalves03 self-assigned this Nov 25, 2024
@nunogoncalves03
Copy link
Member Author

nunogoncalves03 commented Nov 25, 2024

Note that I left some TODO comments regarding whether we should remove the directory-related methods from the FileLocation abstract class. Ideally, both stage and personal/shared spaces would support folders and files; however, currently, personal/shared spaces only support files. Therefore, we have two alternatives:

  1. Remove the directory methods from the abstract class and add them only in the Stage class as additional methods.

  2. Maintain the directory methods in the abstract class to keep it future-proof and more generic, as we plan to implement folder support in personal/shared spaces in the future. In this case, we would return an operation not supported error in the FileSpace class.

Copy link
Collaborator

@kesmit13 kesmit13 left a comment

Choose a reason for hiding this comment

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

I think leaving the unsupported methods in the API and just raising errors is ok. It makes it easier for the user to determine what's wrong if they attempt to use it, although it does make those show up in auto-complete like they are supported. I think the way you have it implemented is the lesser of evils.

singlestoredb/management/files.py Show resolved Hide resolved
singlestoredb/tests/test_management.py Outdated Show resolved Hide resolved
singlestoredb/tests/test_management.py Outdated Show resolved Hide resolved
@nunogoncalves03 nunogoncalves03 force-pushed the ngoncalves/file-location-refactor branch from 2f0a82d to 9f2fc6d Compare December 2, 2024 16:07
@kesmit13 kesmit13 merged commit cd2fbb9 into ngoncalves/file-location-refactor Dec 2, 2024
10 checks passed
@kesmit13 kesmit13 deleted the ngoncalves/files-api-pythonsdk branch December 2, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants