-
Notifications
You must be signed in to change notification settings - Fork 13
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
ArbitraryFileIdManager should have a configurable option to determine its "content root" #43
Comments
Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗 |
Ah, I think I know what you mean. So you're saying that with this setup, the user wouldn't need to pass the same contents root twice to two separate configuration keys? For example,
v.s.
Hm, I'm inclined to agree. Changing the contents root should be as smooth as possible. It requires us to assume the contents root will always be available as an attribute, which I think is OK. |
Correct - although my use case would use a file to contain the config that operators push out. So in your example above, operators would push a config containing:
Then, users
vs. users essentially having to list/update both values:
(I guess I'm also advocating that |
In looking at various ContentsManager implementations (e.g., S3, PostgreSQL) the
root_dir
is not used. Instead, a different (custom) attribute would be tantamount to determining the content root. For example, the referenced S3ContentsManager
has abucket
attribute indicating the bucket in which the content resides. Similarly, the referenced PostgreSQLContentsManager
usesdb_url
to identify the database, which would be sufficient as the content root. (One might argue that theuser_id
attribute may be a better choice, but that's an unrelated discussion.)It seems like
ArbitraryFileIdManager
should introduce a configurable trait that either indicates the name of the attribute from which to pull the content root identifier from or a trait the user must set on both itsContentsManager
andArbitraryFileIdManager
.I'd lean toward the former.
Proposal
content_root_attribute: str
that refers to the attribute on the configuredContentsManager
from which thecontent_root
value (known asroot_dir
today, and that name can continue) is pulled. The default value forcontent_root_attribute
would beroot_dir
.The other option would be to simply expose
root_dir
onArbitraryFileIdManager
which is configured to be the same value as the appropriate attribute onContentsManager
(e.g.,S3ContentsManager.bucket
).The advantage of the first option is that operators/admins would only need to configure the
ArbitraryFileIdManager
once for the duration that that particularContentsManager
is in use. While the second option would require a different configuration for each user. As a result, the first option is helpful for IT staffs that need to push configurations (like in Berkeley's Data 8 class or in PaaS envs).The text was updated successfully, but these errors were encountered: