search

dandi / redirector

Apache License 2.0
0 stars 2 forks source link

Dockerize #16

Closed jwodder closed 4 years ago

jwodder commented 4 years ago

Part of dandi/dandi-cli#170

satra commented 4 years ago

thanks @jwodder - while we are at this, it could be useful to have a github action test here that we can run this script and check for the responses with and without these variables? since the redirector no longer actually queries the service, this should be much more straightforward now.

we could also check the docker build as well. i leave it to you and @yarikoptic whether its done now or in a future PR. otherwise this LGTM.

yarikoptic commented 4 years ago

we could also check the docker build as well. i leave it to you and @yarikoptic whether its done now or in a future PR. otherwise this LGTM.

I like tests! We should hangout! (ref: Idiocracy although not about tests specifically)

@satra -- are these changes go "hot" automagically right upon merge or it requires manual "update" on your end ATM? If go "hot" -- we better add tests right away. Otherwise - IMHO ok to have a separate issue PR. But I will leave it to @jwodder to decide overall.

jwodder commented 4 years ago

Tests do seem like a good idea, but out of scope for this PR.

yarikoptic commented 4 years ago

Filed separate #17 so we do not forget

satra commented 4 years ago

are these changes go "hot" automagically

@yarikoptic - nope. it's a manual restart at the moment. but i'm hoping this will get added to the terraform setup.