Throw error if user tries to upload the same version twice

Signed-off-by: andreasjansson <[email protected]>

Author: andreasjansson

end-to-end-test/end_to_end_test/test_server.py

@@ -124,3 +124,21 @@ def test_push_log(cog_server_port, project_dir):
     assert out["config"]["examples"][2]["output"] == "@cog-example-output/output.02.txt"
     assert out["images"][0]["arch"] == "cpu"
     assert out["images"][0]["run_arguments"]["text"]["type"] == "str"
+
+
+def test_repush(cog_server_port, project_dir):
+    user = random_string(10)
+    model_name = random_string(10)
+    model_url = f"http://localhost:{cog_server_port}/{user}/{model_name}"
+
+    set_model_url(model_url, project_dir)
+    version_id = push_with_log(project_dir)
+
+    _, err = subprocess.Popen(
+        ["cog", "push"],
+        cwd=project_dir,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+    ).communicate()
+
+    assert f"Model version {version_id} already exists" in err.decode()

pkg/client/version_upload.go

@@ -80,7 +80,11 @@ func (c *Client) UploadVersion(mod *model.Model, projectDir string) (*model.Vers
 		return nil, fmt.Errorf("You are not authorized to write to model %s. Did you run cog login?", mod.String())
 	}
 	if resp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("Server returned HTTP status %d", resp.StatusCode)
+		body, err := io.ReadAll(resp.Body)
+		if err != nil {
+			return nil, fmt.Errorf("Failed to read response body: %w", err)
+		}
+		return nil, fmt.Errorf("Push failed. %s", body)
 	}
 
 	var version *model.Version

pkg/server/build.go

@@ -29,26 +29,41 @@ func (s *Server) ReceiveFile(w http.ResponseWriter, r *http.Request) {
 	user, name, _ := getModelVars(r)
 
 	console.Debug("Received build request")
-	streamLogger := logger.NewStreamLogger(r.Context(), w)
-	mod, err := s.ReceiveVersion(r, streamLogger, user, name)
+	dir, err := s.UnzipInputToTempDir(r, user, name)
 	if err != nil {
-		streamLogger.WriteError(err)
+		writeError(w, http.StatusInternalServerError, "Failed to unzip input directory")
 		console.Error(err.Error())
 		return
 	}
-	streamLogger.WriteVersion(mod)
-}
 
-func (s *Server) ReceiveVersion(r *http.Request, logWriter logger.Logger, user string, name string) (*model.Version, error) {
-	dir, err := s.UnzipInputToTempDir(r, user, name)
+	id, err := ComputeID(dir)
 	if err != nil {
-		return nil, err
+		writeError(w, http.StatusInternalServerError, "Failed to compute version ID")
+		console.Error(err.Error())
+		return
+	}
+	version, err := s.db.GetVersion(user, name, id)
+	if err != nil {
+		writeError(w, http.StatusInternalServerError, "Failed to determine if version already exists")
+		console.Error(err.Error())
+		return
+	}
+	if version != nil {
+		writeError(w, http.StatusConflict, "Model version %s already exists", id)
+		return
 	}
 
-	id, err := ComputeID(dir)
+	logWriter := logger.NewStreamLogger(r.Context(), w)
+	mod, err := s.ReceiveVersion(dir, id, logWriter, user, name)
 	if err != nil {
-		return nil, err
+		logWriter.WriteError(err)
+		console.Error(err.Error())
+		return
 	}
+	logWriter.WriteVersion(mod)
+}
+
+func (s *Server) ReceiveVersion(dir string, id string, logWriter logger.Logger, user string, name string) (*model.Version, error) {
 	logWriter.Debugf("Received version %s", id)
 
 	config, err := s.ReadConfig(dir)
@@ -339,3 +354,16 @@ func (s *Server) SendBuildLogs(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 }
+
+func writeError(w http.ResponseWriter, statusCode int, format string, args ...interface{}) {
+	var message string
+	if len(args) > 0 {
+		message = fmt.Sprintf(format, args...)
+	} else {
+		message = format
+	}
+	w.WriteHeader(statusCode)
+	if _, err := w.Write([]byte(message)); err != nil {
+		console.Error("Failed to write to response writer")
+	}
+}