Skip to content

Commit

Permalink
Fix y-sweet path issue (#293)
Browse files Browse the repository at this point in the history
When a y-sweet service is started in a session backend environment (i.e.
`serve-doc`), it expects to receive the document ID as
`SESSION_BACKEND_KEY`.

When an S3 bucket is attached for storage, an environment variable
`STORAGE_PREFIX` is also generated by combining some configured prefix
with the doc ID.

A problem arises because y-sweet already expects to concatenate the doc
ID when doing a lookup. The result is that we end up with the document
ID repeated in the path.

We would still like to respect `STORAGE_PREFIX` if it is provided, but
we want to avoid duplicating the doc ID. This PR:
- Ensures that the last /-delimited component of the `STORAGE_PREFIX` is
the doc ID, unless `STORAGE_PREFIX` is unset or empty
- Chops the doc ID off and constructs a new `STORAGE_PREFIX`.

Also, we no longer require `STORAGE_PREFIX` (since it is allowed to be
empty), but we do not allow `STORAGE_PREFIX` to be set unless
`STORAGE_BUCKET` also is.

I'd like to chop this code up more so that we can unit test it better,
but for now I've tested locally and it solves the issue I was having.
  • Loading branch information
paulgb authored Sep 27, 2024
1 parent 245234f commit 0c925e8
Showing 1 changed file with 30 additions and 19 deletions.
49 changes: 30 additions & 19 deletions crates/y-sweet/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,27 +256,38 @@ async fn main() -> Result<()> {
} => {
let doc_id = env::var("SESSION_BACKEND_KEY").expect("SESSION_BACKEND_KEY must be set");

let bucket = env::var("STORAGE_BUCKET").ok();
let prefix = env::var("STORAGE_PREFIX").ok();

let store = match (bucket, prefix) {
(Some(bucket), Some(prefix)) => {
let prefix = (!prefix.is_empty()).then(|| prefix);
let s3_config = parse_s3_config_from_env_and_args(bucket, prefix)
.context("Failed to parse S3 configuration")?;
let store = S3Store::new(s3_config);
let store: Box<dyn Store> = Box::new(store);
store
.init()
.await
.context("Failed to initialize S3 store")?;
Some(store)
}
(None, None) => {
tracing::warn!("No store set. Documents will be stored in memory only.");
let store = if let Ok(bucket) = env::var("STORAGE_BUCKET") {
let prefix = if let Ok(prefix) = env::var("STORAGE_PREFIX") {
// If the prefix is set, it should contain the document ID as its last '/'-separated part.
// We want to pop that, because we will add it back when accessing the doc.
let mut parts: Vec<&str> = prefix.split('/').collect();
if let Some(last) = parts.pop() {
if last != doc_id {
anyhow::bail!("STORAGE_PREFIX must end with the document ID. Found: {} Expected: {}", last, doc_id);
}

let prefix = parts.join("/");

Some(prefix)
} else {
// As far as y-sweet is concerned, `STORAGE_BUCKET` = "" is equivalent to `STORAGE_BUCKET` not being set.
None
}
} else {
None
};

let s3_config = parse_s3_config_from_env_and_args(bucket, prefix)?;
let store = S3Store::new(s3_config);
let store: Box<dyn Store> = Box::new(store);
store.init().await?;
Some(store)
} else {
if env::var("STORAGE_PREFIX").is_ok() {
anyhow::bail!("If STORAGE_PREFIX is set, STORAGE_BUCKET must also be set.");
}
_ => panic!("Invalid store configuration. Expected both STORAGE_BUCKET and STORAGE_PREFIX environment variables to be set."),

None
};

let cancellation_token = CancellationToken::new();
Expand Down

0 comments on commit 0c925e8

Please sign in to comment.