fix(services/ghac)!: Remove enable_create_simulation support for ghac (…

…#3423) * fix(services/ghac)!: Remove enable_create_simulation support for ghac Signed-off-by: Xuanwo <[email protected]> * Fix ci Signed-off-by: Xuanwo <[email protected]> * Update core/src/docs/upgrade.md Co-authored-by: Suyan <[email protected]> --------- Signed-off-by: Xuanwo <[email protected]> Co-authored-by: Suyan <[email protected]>
apache · Oct 30, 2023 · d77c361 · d77c361
1 parent 6b57b42
commit d77c361
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 36 deletions.
diff --git a/.github/workflows/service_test_ghac.yml b/.github/workflows/service_test_ghac.yml
@@ -60,5 +60,4 @@ jobs:
         run: cargo nextest run behavior --features tests
         env:
           OPENDAL_TEST: ghac
-          OPENDAL_GHAC_ENABLE_CREATE_SIMULATION: true
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/core/src/docs/upgrade.md b/core/src/docs/upgrade.md
@@ -15,6 +15,10 @@ OpenDAL bumps it's MSRV to 1.67.0.
 - The `write_min_size` option has been deprecated and replaced by `BufferedWriter`, also introduced in version 0.40.
 - A new setting, `allow_anonymous`, has been added. Since v0.41, OSS will now return an error if credential loading fails. Enabling `allow_anonymous` to fallback to request without credentials.
 
+### Ghac Service Configuration
+
+- The `enable_create_simulation` option has been removed. We add this option to allow ghac simulate create empty file, but it's could result in unexpected behavior when users create a file with content length `1`. So we remove it.
+
 # Upgrade to v0.41
 
 There is no public API and raw API changes.

diff --git a/core/src/services/ghac/backend.rs b/core/src/services/ghac/backend.rs
@@ -86,7 +86,6 @@ fn value_or_env(
 pub struct GhacBuilder {
     root: Option<String>,
     version: Option<String>,
-    enable_create_simulation: bool,
     endpoint: Option<String>,
     runtime_token: Option<String>,
 
@@ -117,18 +116,6 @@ impl GhacBuilder {
         self
     }
 
-    /// Enable create simulation for ghac service.
-    ///
-    /// ghac service doesn't support create empty files. By enabling
-    /// create simulation, we will create a 1 byte file to represent
-    /// empty file.
-    ///
-    /// As a side effect, we can't create file with only 1 byte anymore.
-    pub fn enable_create_simulation(&mut self) -> &mut Self {
-        self.enable_create_simulation = true;
-        self
-    }
-
     /// Set the endpoint for ghac service.
     ///
     /// For example, this is provided as the `ACTIONS_CACHE_URL` environment variable by the GHA runner.
@@ -175,9 +162,6 @@ impl Builder for GhacBuilder {
 
         map.get("root").map(|v| builder.root(v));
         map.get("version").map(|v| builder.version(v));
-        map.get("enable_create_simulation")
-            .filter(|v| *v == "on" || *v == "true")
-            .map(|_| builder.enable_create_simulation());
 
         builder
     }
@@ -199,7 +183,6 @@ impl Builder for GhacBuilder {
 
         let backend = GhacBackend {
             root,
-            enable_create_simulation: self.enable_create_simulation,
 
             cache_url: value_or_env(self.endpoint.take(), ACTIONS_CACHE_URL, "Builder::build")?,
             catch_token: value_or_env(
@@ -229,7 +212,6 @@ impl Builder for GhacBuilder {
 pub struct GhacBackend {
     // root should end with "/"
     root: String,
-    enable_create_simulation: bool,
 
     cache_url: String,
     catch_token: String,
@@ -278,12 +260,6 @@ impl Accessor for GhacBackend {
         if path.ends_with('/') {
             return Ok(RpCreateDir::default());
         }
-        if !self.enable_create_simulation {
-            return Err(Error::new(
-                ErrorKind::Unsupported,
-                "ghac service doesn't support create empty file",
-            ));
-        }
 
         let req = self.ghac_reserve(path).await?;
 
@@ -404,12 +380,7 @@ impl Accessor for GhacBackend {
         let status = resp.status();
         match status {
             StatusCode::OK => {
-                let mut meta = parse_into_metadata(path, resp.headers())?;
-
-                // Hack for enable_create_simulation.
-                if self.enable_create_simulation && meta.content_length_raw() == Some(1) {
-                    meta.set_content_length(0);
-                }
+                let meta = parse_into_metadata(path, resp.headers())?;
 
                 Ok(RpStat::new(meta))
             }

diff --git a/core/src/types/metadata.rs b/core/src/types/metadata.rs
@@ -185,11 +185,6 @@ impl Metadata {
         self.content_length.unwrap_or_default()
     }
 
-    /// Fetch the raw content length.
-    pub(crate) fn content_length_raw(&self) -> Option<u64> {
-        self.content_length
-    }
-
     /// Set content length of this entry.
     pub fn set_content_length(&mut self, v: u64) -> &mut Self {
         self.content_length = Some(v);