Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

[dev] Use preview namespace for regular workers #1032

Closed
EverlastingBugstopper opened this issue Feb 5, 2020 · 10 comments · Fixed by #1357
Closed

[dev] Use preview namespace for regular workers #1032

EverlastingBugstopper opened this issue Feb 5, 2020 · 10 comments · Fixed by #1357
Labels
feature Feature requests and suggestions
Milestone

Comments

@EverlastingBugstopper
Copy link
Contributor

EverlastingBugstopper commented Feb 5, 2020

For Workers Sites, we create a separate preview KV namespace so that we do not impact production namespaces. We do not do this for regular workers and I believe we should.

In order for folks using wrangler dev or wrangler preview to safely interact with KV namespaces that are not automatically provisioned Workers Sites namespaces, we need to provide some sort of preview namespace.

The most straightforward option here that requires no changes to the Workers API is to do exactly what we already do for Workers Sites: provision a new KV namespace automatically for preview sessions. I've laid out the technical approach for this here.

This solves the immediate goal of stopping impact to production namespaces, but it is not a perfect solution.

Drawbacks

  • Accounts with multiple developers may stomp on each other's preview namespaces during development
  • There may be a lot of churn of preview namespaces, and it will cost developers more money on their KV bill.
  • If we change our approach to preview namespaces in the future, Workers customers who used the feature will have unused preview namespaces lying around in their account, costing them money.

The alternative to my proposed solution is to create a higher-level implementation of preview namespaces.

The best way to do this in my opinion would be to create a true local development environment that allows KV to be mocked and interacted with locally on the developer's machine.

Since this is likely out of the question, the other alternative would be to create a first class idea of preview namespaces with the Workers KV API. This would mean each user (not each account) gets 1 preview namespace per production namespace (hopefully for free). They would be able to interact with those namespaces in the same way that they would with production namespaces. Likely would want to add a --preview flag to all wrangler kv:namespace commands, and when uploading preview workers, the config API would be in charge of replacing the uploaded KV namespace information with a preview namespace. I would guess that a full implementation would also require preview namespaces to be observable in the dashboard to provide a 1:1 developer experience.


My recommendation is to start with the first approach since it is already possible with the API today, and since we already follow that approach for Workers Sites. This will allow us to quickly provide a significant improvement over the current development story, and keep folks from screwing up their production data.

@EverlastingBugstopper EverlastingBugstopper added the feature Feature requests and suggestions label Feb 7, 2020
@EverlastingBugstopper EverlastingBugstopper added this to the dev server milestone Feb 7, 2020
@EverlastingBugstopper EverlastingBugstopper changed the title Use preview namespace for regular workers [dev] Use preview namespace for regular workers Feb 12, 2020
@GregBrimble
Copy link
Member

The option to duplicate data into a new namespace would be great! It would be really useful to let developers spawn a development environment from production without needing to @cloudflare/kv-worker-migrate.

@ashleymichal
Copy link
Contributor

ashleymichal commented Mar 23, 2020

  1. Start creating a namespace automatically on publish if it doesn't exist? We'll have to start doing this on preview and it seems... strange to have different behavior on preview/publish. If we're smart enough on preview, why not publish?

the reason we can do this for sites is that we control the title of the namespace; one mistake i think we made with KV was to use ID instead of Title in the wrangler.toml; Titles are unique per account and therefore would have been better. This particular point would work great if we remedied that by taking a title argument instead of an id argument in the toml, making kv-namespaces top level, and concatenating environment names for envs the way we do for site namespaces. if not, all we have is binding name, and this should not be coupled to title, as the user should be allowed to change it without unexpected effects on their namespace(s), tho perhaps the same could be said for envs.

  1. Add a --preview flag to all of the KV namespace commands?

probably couldn't hurt? tangent: what if preview was a reserved environment name? 🤔

  1. Seed the preview namespace from production data on startup (copy over all the data)?

this is complex and difficult for a few reasons. providing tools for users to do this i think could be valuable but doing so automatically might be overkill.

  1. Create a new namespace automatically on preview sessions based on namespaces defined in wrangler.toml?

this seems good, given the same constraints as my response to point 1

  1. Delete old values in the namespace when the process is killed?

depending on our concerns around churn, we could simply delete preview namespaces when the process is killed; this would be simplest but slowest. making the options around this configurable might be valuable.

@EverlastingBugstopper
Copy link
Contributor Author

This particular point would work great if we remedied that by taking a title argument instead of an id argument in the toml, making kv-namespaces top level, and concatenating environment names for envs the way we do for site namespaces.

unfortunately this has some major backwards incompatibility issues - today we take id and binding and don't allow inheritance from top level.

probably couldn't hurt?

probably would be reasonable addition but would need some way to know if preview session currently exists

what if preview was a reserved environment name

this is an interesting thought, and one that we had discussed in the original environments proposal - dont think it's a good idea here though given backwards compatibility issues and also i'd assume you'd want to use wrangler dev across multiple environments

this is complex and difficult for a few reasons. providing tools for users to do this i think could be valuable but doing so automatically might be overkill.

i think you're right and probably not necessary for mvp

depending on our concerns around churn, we could simply delete preview namespaces when the process is killed; this would be simplest but slowest. making the options around this configurable might be valuable.

probably makes sense just to delete entirely.


The biggest concern I have here is backwards compatibility - changing the way we configure kv namespaces in the pursuit of wrangler dev feels... icky. Perhaps what we can do is

  1. fetch title of kv namespace from id in toml
  2. prepend __preview_ to title and create new namespace
  3. use that namespace w/same binding for preview worker
  4. delete preview namespace when session ends

I think --preview flag on kv commands is probably too complicated and error-prone to pursue, would likely want to print out the title of the namespace we're using and say something like

You can run kv commands using the --title __preview_your_namespace_id flag

@ashleymichal
Copy link
Contributor

unfortunately this has some major backwards incompatibility issues - today we take id and binding and don't allow inheritance from top level.

agreed; this might be a thing to stuff into a 2.0 milestone, which we may want to consider compiling.

the sequence you have is pretty good; it does rely on a clean up step and that really leaves room for odd state problems (what do you do if the preview namespace still exists?)

i think it is worth exploring the use of kv prefixing for this...

@EverlastingBugstopper
Copy link
Contributor Author

EverlastingBugstopper commented Apr 23, 2020

ok so - when a preview session is started with an environment that contains kv-namespaces, we will

  1. fetch the title of the kv namespace from the id in the toml
  2. prepend the title with __preview_ and create the new namespace if it doesn't exist
  3. print the name of the namespace we're using and make sure they know it is separate from production and they may need to repopulate it
  • we don't want to be too prescriptive here because we dont know what they want to do - some users may want to keep the same data in their namespace for all preview sessions because they only write from the worker. some may be testing writes from the worker itself and want the namespace to be empty when it starts. we don't know what they want to do for preview namespaces just like we dont know what they want to do for deployed namespaces, so we need to inform them to run kv commands using the preview namespace we created.
  1. when uploading the worker to the preview api, use the same binding name for the namespace and new namespace id
  2. on teardown, inform the user that their preview namespace still exists and they can delete it if they want to

@EverlastingBugstopper
Copy link
Contributor Author

I've updated the top level comment to reflect the current state of the discussion.

@ispivey
Copy link
Contributor

ispivey commented May 8, 2020

Good feedback/question from @koeninger: "What do I do if I want to preview my production KV namespace?"

@ashleymichal
Copy link
Contributor

ashleymichal commented May 15, 2020

could we add an optional preview field to each kv namespace entry? i would suggest that we follow the steps above, but...

  • first check the namespace for a previewfield and use that id if it exists. this solves the issue of "what if i want to preview my production kv namespace?" otherwise proceed with provisioning the preview namespace for them.
  • if we provision / use a convention-based namespace, include steps to optionally add the preview field to their toml.

playing devil's advocate against my own idea, this could make it look like if you don't add this field we'll use the namespace specified by id. i still prefer having the toml config instead of just a flag though.

@EverlastingBugstopper
Copy link
Contributor Author

EverlastingBugstopper commented May 27, 2020

ok - i've kinda dropped the ball on actually starting implementation on this one/nailing down exact design.

@ashleymichal I really like the idea of being explicit here and adding the idea of a preview namespace to wrangler.toml. i also think we could combine it with environment variables to allow individual developers to each have their own preview namespace, which would solve the problem of having multiple developers squashing each others namespaces.

i think to start out with, we should just flat out refuse to preview if there is not a preview namespace specified for the environment they're using and make them handle the creation/deletion themselves. this makes it so we are not in charge of provisioning namespaces or causing a bunch of churn/inflating their KV billing.

the environment variable config could be added after the fact to add more explicit support for multiple developers, but at first we could just recommend a new environment for each dev.

question becomes - should we also extend the [site] declaration to allow custom preview namespaces to override the one we auto-provision? this is probably out of scope of this particular change but worth revisiting if/when we add support for env variables for multiple devs.

@ashleymichal
Copy link
Contributor

@EverlastingBugstopper this seems like a good call. we can keep our eyes out for optimizations and convenience features, but for solving the biggest problem this seems good.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Feature requests and suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants