cache now implements models.Datastore by just embedding one and then changing
GetApp and GetRoute to have the cache inside. this makes it really flexible
for things like testing, so now the agent doesn't automagically do caching,
now it must be passed a datastore that was wrapped with a cache datastore.
the datastore in the server can remain separate and not use the cache still,
and then now the agent when running fn 'for real' is configured with the cache
baked in. this seems a lot cleaner than what we had and gets the cache out of
the way and it's easier to swap in / out / extend.
something still feels off with this, but i tinkered with it for a day-ish and
didn't come up with anything a whole lot better. doing a lot of the
maneuvering in the caller seemed better but it was just bloating up GetCall so
went back to having it basically like it was, but returning the limited
underlying buffer to read from so we can ship to the db.
some small changes to the LogStore interface, swapped it to take an
io.Reader instead of a string for more flexibility in the future while
essentially maintaining the same level of performance that we have now.
i'm guessing in the not so distant future we'll ship these to some s3 like
service and it would be better to stream them in than carry around a giant
string anyway. also, carrying around up to 1MB buffers in memory isn't great,
we may want to switch to file backed logs for calls, too. using io.Reader for
logs should make #279 more reasonable if/once we move to some s3-like thing,
we can stream from the log storage service direct to clients.
this fixes the span being out of whack and allows the 'right' context to be
used to upload logs (next to inserting the call). deletes the dbWriter we had,
and we just do this in call.End now (which makes sense to me at least).
removes the dupe code for making an stderr for hot / cold and simplifies the
way to get a func logger (no more 7 param methods yay).
closes#298
currently:
* container ran out of memory (code 137)
* container exited with other code != 0
* unable to pull image (auth/404)
there may be others but this is a good start (the most common). notably, for
both hot and cold these should bubble up (if deterministic, which hub isn't
always), and these are useful for users to use in debugging why things aren't
working.
added tests to make sure that these behaviors are working.
also changed the behavior such that when the container exits we return a 502
instead of a 503, just to be able to distinguish the fact that fn is working
as expected but the container is acting funky (400 is weird here, so idk).
removed references to old IsUserVisible crap and slightly changed the
interface for RunResult for plumbing reasons (to get the error type,
specifically).
fixed an issue where if ~/.docker/config.json exists sometimes pulling images
wouldn't work deterministically (should be more inline w/ expectations now)
closes#275
this adds `FN_` in front of env vars that we are injecting into calls, for
namespacing reasons. this will break code relying on the current variables but
if we want to do this, the chance is now really. alternatively, we could
maintain both the old and new for a short period of time to ease the
adjustment (speak now...). updated the docs, as well.
this also adds tests for the notoriously finicky configuration of the env vars
and headers when setting up a call. this won't test the container / request
for the call is actually receiving them, but it's a decent start and will yell
loudly enough upon formatting breakage.
added back FXLB_WAIT to a couple places so the lb can ride again
one thing for feedback:
headers are a bit confusing at the moment (not from this change, but that
behavior is kept here for now), we've a chance to fix them. currently, headers
in the request __are not__ prefixed with `FN_HEADER_`, i.e. 'hot'+sync containers
will receive `Content-Length` in the http request headers, yet a 'cold'
container from the same request would receive `FN_HEADER_Content-Length` in
its environment. This is additionally confusing because if this function were
hot+async, it would receive `FN_HEADER_Content-Length` in the headers, where
just changing it to sync goes back to `Content-Length`. If that was confusing,
then point made ;)
I propose to remove the `FN_HEADER_` prefix for request headers in the
environment, so that the request headers and env will match, as request
headers already are of this format (not prefixed). please lmk thoughts here
Would be fine with going back to the 'plain' vars too, then this patch will
mostly just be adding tests and changing `FN_FORMAT` to `FORMAT`. obviously,
from the examples, it's a bit ingrained now. anyway, entirely up to y'all.
