-
Notifications
You must be signed in to change notification settings - Fork 33
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
chore: Update compose with secondary store setup && minor refactors #270
chore: Update compose with secondary store setup && minor refactors #270
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need to do this in this PR but could we convert our Makefile into a Justfile?
Justfile commands allow embedding shell scripts directly, so we don't need to maintain a whole bunch of separate scripts.
Personally find it much cleaner setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I'm down to use it - lets file an issue
|
||
# Configure MinIO client (mc) | ||
echo "Configuring MinIO client..." | ||
mc alias set local http://minio:9000 minioadmin minioadmin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is mc? I don't have it on my system. Is this an external dependency we're adding? Prob want to document it somewhere. Eventually we'll prob want to have a mise file like op (or go all out and use nix)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oo probably worth noting that this script is only runnable in the minio-init
service since it references the docker network from within
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the goal for this docker-compose file? Is it just for manual testing?
It's not mentioned anywhere in the README, its not in CI, and don't think we use it for anything explicit right now. Might be good to document it and add it to ci if we want to keep it around?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some ideas around this; we coulda add feature testing to ensure lightweight dispersal/retrieval in a more real environment than the one we codify for E2E testing. The idea is you have one script that does a E2E set/get assertion which takes in specific feature flags denoting a specific runtime behavior of proxy which wrap around it; ie:
fallback_dest
: [S3
,redis
,nil
] w/ 3 variationscache_dest
: [S3
,redis
,nil
] w/ 3 variationscert_verification
: [true
,false
] w 2 variations
3 * 3 * 2 = 18 permutations; but less since `fallback_dest` and `cache_dest` can only be sampled with replacement for `nil` so it'd be - ( 2 * 2) or:
18 - 4 = 14 test case permutations
Think this would be good to have since it also provides a more infra level overview of what wiring proxy with dependencies look like. This would definitely help devx when people have to spin-up additional infra and wire it with proxy.
Fixes Issue
Fixes #
Changes proposed
Screenshots (Optional)
Note to reviewers