Skip to content

Commit afca975

Browse files
captbaritonefacebook-github-bot
authored andcommitted
Allow required alias codemod to be applied to a rollout range
Reviewed By: gordyf Differential Revision: D69144209 fbshipit-source-id: cf5cd4d933fadfbe84159266153459e3e0827de2
1 parent 40b579f commit afca975

File tree

6 files changed

+1820
-54
lines changed

6 files changed

+1820
-54
lines changed

compiler/crates/common/src/feature_flags.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use schemars::JsonSchema;
1616
use serde::Deserialize;
1717
use serde::Serialize;
1818

19+
use crate::rollout::RolloutRange;
1920
use crate::Rollout;
2021

2122
#[derive(Default, Debug, Serialize, Deserialize, Clone, JsonSchema)]
@@ -183,6 +184,9 @@ pub enum FeatureFlag {
183184

184185
/// Partially enabled: used for gradual rollout of the feature
185186
Rollout { rollout: Rollout },
187+
188+
/// Partially enabled: used for gradual rollout of the feature
189+
RolloutRange { rollout: RolloutRange },
186190
}
187191

188192
impl FeatureFlag {
@@ -191,6 +195,7 @@ impl FeatureFlag {
191195
FeatureFlag::Enabled => true,
192196
FeatureFlag::Limited { allowlist } => allowlist.contains(&name),
193197
FeatureFlag::Rollout { rollout } => rollout.check(name.lookup()),
198+
FeatureFlag::RolloutRange { rollout } => rollout.check(name.lookup()),
194199
FeatureFlag::Disabled => false,
195200
}
196201
}
@@ -211,6 +216,7 @@ impl Display for FeatureFlag {
211216
f.write_str(&items.join(", "))
212217
}
213218
FeatureFlag::Rollout { rollout } => write!(f, "Rollout: {:#?}", rollout),
219+
FeatureFlag::RolloutRange { rollout } => write!(f, "RolloutRange: {:#?}", rollout),
214220
}
215221
}
216222
}

compiler/crates/common/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,5 +63,6 @@ pub use perf_logger::PerfLogEvent;
6363
pub use perf_logger::PerfLogger;
6464
pub use pointer_address::PointerAddress;
6565
pub use rollout::Rollout;
66+
pub use rollout::RolloutRange;
6667
pub use span::Span;
6768
pub use text_source::TextSource;

compiler/crates/common/src/rollout.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,23 @@ impl Rollout {
3131
}
3232
}
3333
}
34+
35+
/// A utility to enable gradual rollout of large codegen changes. Allows you to
36+
/// specify a range of percentages to rollout.
37+
#[derive(Debug, Serialize, Deserialize, Clone, Copy, JsonSchema)]
38+
pub struct RolloutRange {
39+
pub start: u8,
40+
pub end: u8,
41+
}
42+
43+
impl RolloutRange {
44+
/// Checks some key deterministically and passes on average the given
45+
/// percentage of the rollout.
46+
/// A typical key to pass in could be the fragment or operation name.
47+
pub fn check(&self, key: impl AsRef<[u8]>) -> bool {
48+
let hash = Md5::digest(key.as_ref());
49+
let hash: u16 = ((hash[1] as u16) << 8) | (hash[0] as u16);
50+
let percent = hash % 100;
51+
(percent) <= (self.end as u16) && (percent) >= (self.start as u16)
52+
}
53+
}

compiler/crates/relay-codegen/src/build_ast.rs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use ::intern::string_key::Intern;
1212
use ::intern::string_key::StringKey;
1313
use ::intern::Lookup;
1414
use common::DirectiveName;
15-
use common::FeatureFlag;
1615
use common::NamedItem;
1716
use common::ObjectName;
1817
use common::WithLocation;
@@ -2394,17 +2393,11 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
23942393
kind: Primitive::String(CODEGEN_CONSTANTS.module_import),
23952394
};
23962395

2397-
let should_use_reader_module_imports =
2398-
match &self.project_config.feature_flags.use_reader_module_imports {
2399-
FeatureFlag::Enabled => true,
2400-
FeatureFlag::Disabled => false,
2401-
FeatureFlag::Limited {
2402-
allowlist: fragment_names,
2403-
} => fragment_names.contains(&module_metadata.key),
2404-
FeatureFlag::Rollout { rollout } => {
2405-
rollout.check(module_metadata.key.lookup().as_bytes())
2406-
}
2407-
};
2396+
let should_use_reader_module_imports = self
2397+
.project_config
2398+
.feature_flags
2399+
.use_reader_module_imports
2400+
.is_enabled_for(module_metadata.key);
24082401

24092402
match self.variant {
24102403
CodegenVariant::Reader => {

compiler/crates/relay-codemod/src/codemod.rs

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use clap::Args;
1212
use clap::Subcommand;
1313
use common::FeatureFlag;
1414
use common::Rollout;
15+
use common::RolloutRange;
1516
use log::info;
1617
use lsp_types::CodeActionOrCommand;
1718
use lsp_types::TextEdit;
@@ -32,9 +33,11 @@ pub enum AvailableCodemod {
3233

3334
#[derive(Args, Debug, Clone)]
3435
pub struct MarkDangerousConditionalFragmentSpreadsArgs {
35-
/// If using a feature flag, specify the rollout percentage. If omitted, the codemod is fully enabled.
36-
#[clap(long, short, value_parser=valid_percent)]
37-
pub rollout_percentage: Option<u8>,
36+
/// Specify a percentage of fragments to codemod. If a number is provided,
37+
/// the first n percentage of fragments will be codemodded. If a range (`20-30`) is
38+
/// provided, then fragments between the start and end of the range will be codemodded.
39+
#[clap(long, short, value_parser=valid_percent, default_value = "100")]
40+
pub rollout_percentage: FeatureFlag,
3841
}
3942

4043
pub async fn run_codemod(
@@ -46,12 +49,7 @@ pub async fn run_codemod(
4649
.iter()
4750
.flat_map(|programs| match &codemod {
4851
AvailableCodemod::MarkDangerousConditionalFragmentSpreads(opts) => {
49-
match fragment_alias_directive(
50-
&programs.source,
51-
&FeatureFlag::Rollout {
52-
rollout: Rollout(opts.rollout_percentage),
53-
},
54-
) {
52+
match fragment_alias_directive(&programs.source, &opts.rollout_percentage) {
5553
Ok(_) => vec![],
5654
Err(e) => e,
5755
}
@@ -143,13 +141,33 @@ fn sort_changes(url: &Url, changes: &mut Vec<TextEdit>) -> Result<(), std::io::E
143141
Ok(())
144142
}
145143

146-
fn valid_percent(s: &str) -> Result<u8, String> {
147-
// turn s into a u8
148-
let s = s.parse::<u8>().map_err(|_| "not a number".to_string())?;
149-
// check if s is less than 100
150-
if (0..=100).contains(&s) {
151-
Ok(s)
144+
fn valid_percent(s: &str) -> Result<FeatureFlag, String> {
145+
// If the string is a range of the form "x-y", where x and y are numbers, return the range
146+
let parts: Vec<&str> = s.split('-').collect();
147+
if parts.len() == 2 {
148+
let start = parts[0].parse::<u8>().map_err(|_| {
149+
"Expected the value on the left of the rollout range to be a number".to_string()
150+
})?;
151+
let end = parts[1].parse::<u8>().map_err(|_| {
152+
"Expected the value on the right of the rollout range to be a number".to_string()
153+
})?;
154+
if (0..=100).contains(&start) && (0..=100).contains(&end) && start <= end {
155+
Ok(FeatureFlag::RolloutRange {
156+
rollout: RolloutRange { start, end },
157+
})
158+
} else {
159+
Err("numbers must be between 0 and 100, inclusive, and the first number must be less than or equal to the second".to_string())
160+
}
152161
} else {
153-
Err("number must be between 0 and 100, inclusive".to_string())
162+
// turn s into a u8
163+
let s = s.parse::<u8>().map_err(|_| "not a number".to_string())?;
164+
// check if s is less than 100
165+
if (0..=100).contains(&s) {
166+
Ok(FeatureFlag::Rollout {
167+
rollout: Rollout(Some(s)),
168+
})
169+
} else {
170+
Err("number must be between 0 and 100, inclusive".to_string())
171+
}
154172
}
155173
}

0 commit comments

Comments
 (0)