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.

Sign up for GitHub

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

Jump to bottom

feat: changes name of delete command #159

Closed
wants to merge 5 commits into from
Closed

feat: changes name of delete command #159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0194a42
feat: changes name of delete command
Jul 7, 2022
ab49efa
feat: adds new delete function to CLI
Jul 7, 2022
1b37ae9
chore: cleans lint
Jul 7, 2022
fa2bf39
chore: solves build failure
Jul 7, 2022
7fc802e
chore: solves build failure
Jul 11, 2022
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
4 changes: 2 additions & 2 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ toml = "0.5.8"
maplit = "1.0.2"
configparser = "3.0.0"
regex = "1"
momento = "0.5.4"
momento = "0.6.0"
qrcode = "0.12.0"
webbrowser = "0.7.1"

Expand Down
7 changes: 7 additions & 0 deletions src/commands/cache/cache_cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,10 @@ pub async fn get(cache_name: String, auth_token: String, key: String) -> Result<
};
Ok(())
}

pub async fn delete(cache_name: String, auth_token: String, key: String) -> Result<(), CliError> {
debug!("getting key: {} from cache: {}", key, cache_name);

let mut client = get_momento_client(auth_token).await?;
interact_with_momento("getting...", client.delete(&cache_name, key)).await
}
36 changes: 30 additions & 6 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ enum CloudSignupCommand {
#[derive(Debug, StructOpt)]
enum CacheCommand {
#[structopt(about = "Create a cache")]
Create {
CreateCache {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this would make people need to type

momento cache createcache --name cachename

right? I'm not convinced that writing cache again right after writing cache is removing any lack of clarity that already exists. Can you explain the motivation behind this change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kvcache he was going directly off this ticket per @danielamiao comments. Im updating pr w/ ticket link as well now.
#145

I know we had talked as well to collapse commands down to root of CLI but would rather that be follow up pr then after this initial refactor for ticket @WillRInternship working on now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also this boils down to momento cache create-cache --name cachename with how clap parses commands currently just FYI it adds in the -

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not aligned... let's take this to slack or zoom.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok we clarified w/ @danielamiao we want to move down to root. @WillRInternship check out comment here.
#145 (comment)

Id still from implementation wise get delete added first make sure works as you would expect. Then can make a follow up commit to get rid of the cache subcommand and move all the commands down to root of CLI like @danielamiao saying in ticket.

#[structopt(long = "name", short = 'n')]
cache_name: String,
#[structopt(long, short, default_value = "default")]
Expand Down Expand Up @@ -148,15 +148,26 @@ enum CacheCommand {
},

#[structopt(about = "Delete the cache")]
Delete {
DeleteCache {
#[structopt(long = "name", short = 'n')]
cache_name: String,
#[structopt(long, short, default_value = "default")]
profile: String,
},

#[structopt(about = "Delete an item from the cache")]
Delete {
#[structopt(long = "name", short = 'n')]
cache_name: Option<String>,
// TODO: Add support for non-string key-value
#[structopt(long, short)]
key: String,
#[structopt(long, short, default_value = "default")]
profile: String,
},

#[structopt(about = "List all caches")]
List {
ListCaches {
#[structopt(long, short, default_value = "default")]
profile: String,
},
Expand All @@ -176,7 +187,7 @@ async fn entrypoint() -> Result<(), CliError> {

match args.command {
Subcommand::Cache { operation } => match operation {
CacheCommand::Create {
CacheCommand::CreateCache {
cache_name,
profile,
} => {
Expand Down Expand Up @@ -214,18 +225,31 @@ async fn entrypoint() -> Result<(), CliError> {
)
.await?;
}
CacheCommand::Delete {
CacheCommand::DeleteCache {
cache_name,
profile,
} => {
let (creds, _config) = get_creds_and_config(&profile).await?;
commands::cache::cache_cli::delete_cache(cache_name.clone(), creds.token).await?;
debug!("deleted cache {}", cache_name)
}
CacheCommand::List { profile } => {
CacheCommand::ListCaches { profile } => {
let (creds, _config) = get_creds_and_config(&profile).await?;
commands::cache::cache_cli::list_caches(creds.token).await?
}
CacheCommand::Delete {
cache_name,
key,
profile,
} => {
let (creds, config) = get_creds_and_config(&profile).await?;
commands::cache::cache_cli::delete(
cache_name.unwrap_or(config.cache),
creds.token,
key,
)
.await?;
}
},
Subcommand::Configure { quick, profile } => {
commands::configure::configure_cli::configure_momento(quick, &profile).await?
Expand Down
2 changes: 1 addition & 1 deletion tests/configure_profiles_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ mod tests {
async fn momento_cache_delete_default_profile() {
let test_cache_default = std::env::var("TEST_CACHE_DEFAULT").unwrap();
let mut cmd = Command::cargo_bin("momento").unwrap();
cmd.args(&["cache", "delete", "--name", &test_cache_default])
cmd.args(&["cache", "delete-cache", "--name", &test_cache_default])
.assert()
.success();
}
Expand Down
6 changes: 3 additions & 3 deletions tests/momento_additional_profile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ mod tests {
let mut cmd = Command::cargo_bin("momento").unwrap();
cmd.args(&[
"cache",
"create",
"create-cache",
"--name",
&test_cache_with_profile,
"--profile",
Expand Down Expand Up @@ -49,7 +49,7 @@ mod tests {
test_cache_with_profile.push('\n');
let test_profile = std::env::var("TEST_PROFILE").unwrap();
let mut cmd = Command::cargo_bin("momento").unwrap();
cmd.args(&["cache", "list", "--profile", &test_profile])
cmd.args(&["cache", "list-caches", "--profile", &test_profile])
.assert()
.stdout(test_cache_with_profile);
}
Expand All @@ -60,7 +60,7 @@ mod tests {
let mut cmd = Command::cargo_bin("momento").unwrap();
cmd.args(&[
"cache",
"delete",
"delete-cache",
"--name",
&test_cache_with_profile,
"--profile",
Expand Down
10 changes: 5 additions & 5 deletions tests/momento_default_profile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ mod tests {
let test_cache_default = std::env::var("TEST_CACHE_DEFAULT").unwrap();
let output = Command::cargo_bin("momento")
.unwrap()
.args(&["cache", "list"])
.args(&["cache", "list-caches"])
.output()
.unwrap()
.stdout;
Expand All @@ -19,13 +19,13 @@ mod tests {
if !cache.is_empty() {
Command::cargo_bin("momento")
.unwrap()
.args(&["cache", "delete", "--name", cache])
.args(&["cache", "delete-cache", "--name", cache])
.unwrap();
}
}
}
let mut cmd = Command::cargo_bin("momento").unwrap();
cmd.args(&["cache", "create", "--name", &test_cache_default])
cmd.args(&["cache", "create-cache", "--name", &test_cache_default])
.assert()
.success();
}
Expand All @@ -48,15 +48,15 @@ mod tests {
let mut test_cache_default = std::env::var("TEST_CACHE_DEFAULT").unwrap();
test_cache_default.push('\n');
let mut cmd = Command::cargo_bin("momento").unwrap();
cmd.args(&["cache", "list"])
cmd.args(&["cache", "list-caches"])
.assert()
.stdout(test_cache_default);
}

async fn momento_cache_delete_default_profile() {
let test_cache_default = std::env::var("TEST_CACHE_DEFAULT").unwrap();
let mut cmd = Command::cargo_bin("momento").unwrap();
cmd.args(&["cache", "delete", "--name", &test_cache_default])
cmd.args(&["cache", "delete-cache", "--name", &test_cache_default])
.assert()
.success();
}
Expand Down