Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/service_test_ghac.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
4 changes: 4 additions & 0 deletions core/src/docs/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
31 changes: 1 addition & 30 deletions core/src/services/ghac/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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))
}
Expand Down
5 changes: 0 additions & 5 deletions core/src/types/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading