Copy pared down prefect-redis to integrations
#13758
Conversation
| from prefect.filesystems import WritableFileSystem | ||
|
|
||
|
|
||
| class RedisFilesystem(WritableFileSystem): |
There was a problem hiding this comment.
I know this is copying what currently exists, but I think we should change the name since RedisFilesystem seems paradoxical. Doesn't need to be part of this PR.
There was a problem hiding this comment.
The original name was actually 'RedisCredentials'. This is the renamed version, RedisFilesystem is what the issue suggested.
In what way do you think it's paradoxical? The point of the block is to treat redis as a filesystem, much the same way GCS or S3 work, so the name makes sense to me, but happy to consider alternatives.
There was a problem hiding this comment.
The tests are failing for this because it doesn't have redis running in CI. But I'm going to go ahead and merge this, and have a follow-up to get those tests running. We can talk about naming, etc. in slack.
There was a problem hiding this comment.
I thought that Redis isn't an object storage solution like S3 and GCS that uses file-like semantics, so it feels weird to call it a filesystem. I guess we've backed ourselves into a corner with the Filesystem interface name. My vote would be to name it RedisDatabase or something like that since other classes that implement a Filesystem interface are named things like S3Bucket and AzureBlobStorageContainer to try and make it clear how they map onto external concepts.
There was a problem hiding this comment.
Right, It's not an object storage system in general, at least not like S3/GCS, I was meaning that this class is treating redis as if it were like those in that it's letting you store 'file content' (bytes) associated with a 'path' (a key) much the same as GCS works.
This copies over a pared down / refactored version of
prefect-redisthat only includes the filesystem capabilities and full test coverage.There are a couple things left for follow-up PRs:
Related to #12249