[narimonf]

This PR regenerate the client from the latest release protos.

Adds support for TVC app region:

• tvc app create takes a region (region in app.json or --region); the default is us-east-1, the other allowed value is ap-southeast-1
• tvc app list shows each app's region

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[richardpringle]

Only real requested change is the first comment

Region should implement Deserialize

Which would also simplify the tests if you want to just test Region, but 🤷 the test coverage is there regardless.

In any case, it would be better to rely on serde's derive-macro than writing a custom function that unnecessarily allocates.

The other comments are options, take them or leave them. From the perspective of this code, it would be a lot better to use us-east1 even though AWS uses the second dash (which I do think looks better) because you can just add regions as concrete Rust types without worrying about making a mistake. If the backend already relies on the second dash, I guess that's fine too, just more code here.

[richardpringle]

This is an anti-pattern. The type should implement Deserialize instead.

[richardpringle]

If you drop the second dash, you get a bunch of code for free:

Suggested change

	/// Region a TVC app runs in, as accepted by app creation.
	///
	/// Closed on purpose: a region the server adds needs a new variant here
	/// before the CLI can request it.
	#[derive(Debug, Clone, Copy, PartialEq, Default, Serialize, ValueEnum)]
	pub enum Region {
	/// US East (N. Virginia). The default.
	#[default]
	#[serde(rename = "us-east-1")]
	#[value(name = "us-east-1")]
	UsEast1,
	/// Asia Pacific (Singapore). Requires the organization to be enabled for it.
	#[serde(rename = "ap-southeast-1")]
	#[value(name = "ap-southeast-1")]
	ApSoutheast1,
	}
	impl Display for Region {
	fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
	f.write_str(match self {
	Self::UsEast1 => "us-east-1",
	Self::ApSoutheast1 => "ap-southeast-1",
	})
	}
	/// Region a TVC app runs in, as accepted by app creation.
	///
	/// Closed on purpose: a region the server adds needs a new variant here
	/// before the CLI can request it.
	#[derive(
	Debug,
	Clone,
	Copy,
	PartialEq,
	Default,
	Serialize,
	Deserialize,
	IntoStaticStr,
	EnumString,
	VariantArray,
	)]
	#[serde(rename_all = "kebab-case")]
	#[strum(serialize_all="kebab-case", parse_err_fn = ToString::to_string, parse_err_ty = String)]
	/// See [ValueEnum] implementation for help text details
	pub enum Region {
	#[default
	UsEast1, // if you need the -1, you can rename with both strum and serde
	ApSoutheast1,
	}
	impl ValueEnum for Region {
	fn value_variants<'a>() -> &'a [Self] {
	&Self::VARIANTS
	}
	fn to_possible_value(&self) -> Option<clap::builder::PossibleValue> {
	let value: &'static str = self.into();
	let help_text = match self {
	Region::UsEast1 => "US East (N. Virginia). The default",
	Region::ApSoutheast1 => {
	"Asia Pacific (Singapore). Requires the organization to be enabled for it"
	}
	};
	clap::builder::PossibleValue::new(value)
	.help(help_text)
	.into()
	}
	// better to have strum handle this than the default clap implementation
	fn from_str(input: &str, ignore_case: bool) -> std::result::Result<Self, String> {
	let input = if ignore_case {
	Cow::from(input.to_lowercase())
	} else {
	Cow::from(input)
	};
	input.parse()
	}
	}
	impl Display for Region {
	fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
	f.write_str(self.into())
	}

If you do this, you'll also have to make a few changes to the tests:

diff --git a/tvc/Cargo.toml b/tvc/Cargo.toml
index d1fabd93..b5aeb454 100644
--- a/tvc/Cargo.toml
+++ b/tvc/Cargo.toml
@@ -49,6 +49,7 @@ zeroize = { workspace = true }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true }
 displaydoc = { workspace = true }
+strum.workspace = true
 
 [dev-dependencies]
 assert_cmd = { workspace = true }
diff --git a/tvc/src/commands/app/create.rs b/tvc/src/commands/app/create.rs
index ade8b978..51c8735f 100644
--- a/tvc/src/commands/app/create.rs
+++ b/tvc/src/commands/app/create.rs
@@ -510,8 +510,8 @@ mod tests {
     #[test]
     fn build_intent_forwards_region_from_config() {
         for (region, expected) in [
-            (Region::UsEast1, "us-east-1"),
-            (Region::ApSoutheast1, "ap-southeast-1"),
+            (Region::UsEast1, "us-east1"),
+            (Region::ApSoutheast1, "ap-southeast1"),
         ] {
             let mut config = valid_config();
             config.region = region;
@@ -563,7 +563,7 @@ mod tests {
             config_path,
             "--dangerous-enable-debug-mode-deployments",
             "--region",
-            "ap-southeast-1",
+            "ap-southeast1",
         ])
         .unwrap()
         .args;
diff --git a/tvc/src/config/app.rs b/tvc/src/config/app.rs
index 08d93ca4..73f5d3da 100644
--- a/tvc/src/config/app.rs
+++ b/tvc/src/config/app.rs
@@ -423,8 +447,8 @@ mod tests {
         assert_eq!(config.region, Region::UsEast1);
 
         let mut json = valid_config_json();
-        json["region"] = json!("ap-southeast-1");
-        let config: AppConfig = serde_json::from_value(json).unwrap();
+        json["region"] = json!("ap-southeast1");
+        let config: AppConfig = dbg!(serde_json::from_value(json)).unwrap();
 
         assert_eq!(config.region, Region::ApSoutheast1);
     }
@@ -432,14 +456,14 @@ mod tests {
     #[test]
     fn config_rejects_unknown_region() {
         let mut json = valid_config_json();
-        json["region"] = json!("eu-west-1");
+        json["region"] = json!("eu-west1");
 
         let error = serde_json::from_value::<AppConfig>(json).unwrap_err();
 
         assert!(error.is_data());
         assert_eq!(
             error.to_string(),
-            "region: unknown value `eu-west-1`, expected `us-east-1` or `ap-southeast-1`"
+            "unknown variant `eu-west1`, expected `us-east1` or `ap-southeast1`"
         );
     }
 
@@ -451,10 +475,6 @@ mod tests {
         let error = serde_json::from_value::<AppConfig>(json).unwrap_err();
 
         assert!(error.is_data());
-        assert_eq!(
-            error.to_string(),
-            "invalid type: integer `1`, expected a string"
-        );
     }
 
     /// The config file, the CLI flag, and the API request must all spell each

[richardpringle]

asserting on the exact string is a maintenance burden. I would just check that the string contains the possible value and the invalid value