add deprecation warnings, failure case, feature header

src/commands/publish.rs

@@ -27,6 +27,7 @@ pub fn publish(
         let path = &site_config.bucket.clone();
         validate_bucket_location(path)?;
         warn_site_incompatible_route(&deploy_config);
+
         let site_namespace = add_site_namespace(user, target, false)?;
 
         let (to_upload, to_delete, asset_manifest) =
@@ -38,7 +39,6 @@ pub fn publish(
         }
         upload_files(target, user, &site_namespace.id, to_upload)?;
 
-        sync_other_buckets(target, user, verbose)?;
         let upload_client = http::auth_client(Some("site"), user);
 
         // Next, upload and deploy the worker with the updated asset_manifest
@@ -55,9 +55,16 @@ pub fn publish(
             delete_bulk(target, user, &site_namespace.id, to_delete)?;
         }
     } else {
-        sync_other_buckets(target, user, verbose)?;
+        let uses_kv_bucket = sync_non_site_buckets(target, user, verbose)?;
+
+        let feature = if uses_kv_bucket {
+            message::deprecation_warning("As of 1.9.0, you will no longer be able to specify a `bucket` for a kv namespace in your `wrangler.toml`. If your application depends on this feature, please file an issue at https://github.com/cloudflare/wrangler/issues/new with your use case.https://github.com/cloudflare/wrangler/issues/new?labels=user+report&template=feature_request.md");
+            Some("bucket")
+        } else {
+            None
+        };
 
-        let upload_client = http::auth_client(None, user);
+        let upload_client = http::auth_client(feature, user);
         upload::script(&upload_client, &target, None)?;
 
         deploy::worker(&user, &deploy_config)?;
@@ -98,6 +105,10 @@ pub fn add_site_namespace(
     for namespace in target.kv_namespaces() {
         if namespace.id == site_namespace.id {
             return Ok(namespace); // Sites binding already exists; ignore
+        } else {
+            if namespace.bucket.is_some() {
+               failure::bail!("your wrangler.toml includes a `bucket` as part of a kv_namespace but also has a `[site]` specifed; did you mean to put this under `[site]`?");
+            }
         }
     }
 
@@ -141,43 +152,45 @@ pub fn validate_bucket_location(bucket: &PathBuf) -> Result<(), failure::Error>
     Ok(())
 }
 
-pub fn sync_other_buckets(
+// This is broken into a separate step because the intended design does not
+// necessarily intend for bucket support outside of the [site] usage, especially
+// since assets are still hashed. In a subsequent release, we will either
+// deprecate this step, or we will integrate it more closely and adapt to user
+// feedback.
+//
+// In order to track usage of this "feature", this function returns a bool that
+// indicates whether any non-site kv namespaces were specified / uploaded.
+pub fn sync_non_site_buckets(
     target: &Target,
     user: &GlobalUser,
     verbose: bool,
-) -> Result<(), failure::Error> {
-    let site_bucket = if let Some(site_config) = &target.site {
-        Some(PathBuf::from(&site_config.bucket))
-    } else {
-        None
-    };
+) -> Result<bool, failure::Error> {
+    let mut is_using_non_site_bucket = false;
 
     for namespace in target.kv_namespaces().iter() {
-        if namespace.bucket != site_bucket {
-            if let Some(path) = &namespace.bucket {
-                // TODO: add deprecation warning
-                validate_bucket_location(path)?;
-                let (to_upload, to_delete, _) =
-                    kv::bucket::sync(target, user, &namespace.id, path, verbose)?;
-                // First, upload all existing files in bucket directory
+        if let Some(path) = &namespace.bucket {
+            is_using_non_site_bucket = true;
+            validate_bucket_location(path)?;
+            let (to_upload, to_delete, _) =
+                kv::bucket::sync(target, user, &namespace.id, path, verbose)?;
+            // First, upload all existing files in bucket directory
+            if verbose {
+                message::info("Preparing to upload updated files...");
+            }
+            upload_files(target, user, &namespace.id, to_upload)?;
+
+            // Finally, remove any stale files
+            if !to_delete.is_empty() {
                 if verbose {
-                    message::info("Preparing to upload updated files...");
+                    message::info("Deleting stale files...");
                 }
-                upload_files(target, user, &namespace.id, to_upload)?;
 
-                // Finally, remove any stale files
-                if !to_delete.is_empty() {
-                    if verbose {
-                        message::info("Deleting stale files...");
-                    }
-
-                    delete_bulk(target, user, &namespace.id, to_delete)?;
-                }
+                delete_bulk(target, user, &namespace.id, to_delete)?;
             }
         }
     }
 
-    Ok(())
+    Ok(is_using_non_site_bucket)
 }
 
 fn validate_target_required_fields_present(target: &Target) -> Result<(), failure::Error> {

src/http.rs

@@ -1,7 +1,8 @@
+use std::time::Duration;
+
 use reqwest::blocking::{Client, ClientBuilder};
 use reqwest::header::{HeaderMap, HeaderValue, USER_AGENT};
 use reqwest::redirect::Policy;
-use std::time::Duration;
 
 use crate::install;
 use crate::settings::global_user::GlobalUser;

src/terminal/message.rs

@@ -44,3 +44,8 @@ pub fn help(msg: &str) {
     let msg = format!("{} {}", emoji::SLEUTH, msg);
     message(&msg);
 }
+
+pub fn deprecation_warning(msg: &str) {
+    let msg = format!("\n\t{} {}", emoji::WARN, msg);
+    message(&msg);
+}