let the server provide the default cp cmd version

beam-cloud · Jan 9, 2025 · f6afe5f · f6afe5f
1 parent c1abcf2
commit f6afe5f
Show file tree

Hide file tree

Showing 8 changed files with 185 additions and 178 deletions.
diff --git a/pkg/abstractions/volume/multipart.go b/pkg/abstractions/volume/multipart.go
@@ -50,8 +50,9 @@ func joinCleanPath(parts ...string) string {
 
 func (s *GlobalVolumeService) GetFileServiceInfo(ctx context.Context, in *pb.GetFileServiceInfoRequest) (*pb.GetFileServiceInfoResponse, error) {
 	return &pb.GetFileServiceInfoResponse{
-		Ok:      true,
-		Enabled: s.config.EndpointURL != "" && s.config.BucketName != "",
+		Ok:             true,
+		Enabled:        s.config.EndpointURL != "" && s.config.BucketName != "",
+		CommandVersion: s.config.CommandVersion,
 	}, nil
 }
 

diff --git a/pkg/abstractions/volume/volume.proto b/pkg/abstractions/volume/volume.proto
@@ -140,6 +140,7 @@ message GetFileServiceInfoResponse {
   bool ok = 1;
   string err_msg = 2;
   bool enabled = 3;
+  uint32 command_version = 4;
 }
 
 message CreatePresignedURLRequest {

diff --git a/pkg/common/config.default.yaml b/pkg/common/config.default.yaml
@@ -62,6 +62,7 @@ fileService:
   accessKey: test
   secretKey: test-test-test
   region:
+  commandVersion: 1
 imageService:
   localCacheEnabled: true
   registryStore: local

diff --git a/pkg/types/config.go b/pkg/types/config.go
@@ -129,6 +129,9 @@ type FileServiceConfig struct {
 	AccessKey   string `key:"accessKey" json:"access_key"`
 	SecretKey   string `key:"secretKey" json:"secret_key"`
 	Region      string `key:"region" json:"region"`
+
+	// Determines which version of the copy command to use in the CLI
+	CommandVersion uint32 `key:"commandVersion" json:"command_version"`
 }
 
 type ImageServiceConfig struct {

diff --git a/proto/volume.pb.go b/proto/volume.pb.go
diff --git a/sdk/src/beta9/cli/volume.py b/sdk/src/beta9/cli/volume.py
@@ -252,8 +252,9 @@ def read_with_progress(
     "--version",
     is_eager=True,
     type=click.Choice(["1", "2"]),
-    help="Version of this command.",
-    default="2",
+    help="The version of this command to run. [default is determined by the server]",
+    default=None,
+    required=False,
 )
 @extraclick.pass_service_client
 def cp(
@@ -262,13 +263,10 @@ def cp(
     destination: Union[Path, RemotePath],
     version: str,
 ):
-    if version == "1":
-        terminal.warn("v1 of the cp command is deprecated. Use v2 instead.")
-        return cp_v1(service, source, destination)  # type: ignore
-
     res = service.volume.get_file_service_info(GetFileServiceInfoRequest())
-    if not res.ok or not res.enabled:
-        terminal.warn("Multipart upload/download is not enabled. Falling back to v1.")
+    if version == "1" or (version is None and res.command_version == 1) or not res.enabled:
+        if "://" in str(source) or "://" in str(destination):
+            raise click.BadArgumentUsage("Remote path syntax ($scheme://) is not supported in v1.")
         return cp_v1(service, source, destination)  # type: ignore
 
     if isinstance(source, Path) and isinstance(destination, Path):

diff --git a/sdk/src/beta9/clients/volume/__init__.py b/sdk/src/beta9/clients/volume/__init__.py
diff --git a/sdk/src/beta9/multipart.py b/sdk/src/beta9/multipart.py
@@ -581,17 +581,8 @@ def convert(
         param: Optional[click.Parameter] = None,
         ctx: Optional[click.Context] = None,
     ) -> Union[str, Path, RemotePath]:
-        if (
-            ctx is not None
-            and (version := ctx.params.get(self.version_option_name))
-            and version == "1"
-        ):
-            # Don't convert to RemotePath for v1
-            return value
-
         if "://" in value:
             return RemotePath.parse(value)
-
         return Path(value)