From cde4ea2b39fd38030b702c3c17d349e608671bfb Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 10 Sep 2024 01:43:26 +0300 Subject: [PATCH] Add --timeline-id option to "neon_local timeline branch" command Makes it consistent with the "timeline create" and "timeline import" commands, which allowed you to pass the timeline id as argument. This also makes it unnecessary to parse the timeline ID from the output in the python function that calls it. --- control_plane/src/bin/neon_local.rs | 4 ++- test_runner/fixtures/neon_fixtures.py | 42 ++++++++++----------------- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/control_plane/src/bin/neon_local.rs b/control_plane/src/bin/neon_local.rs index 1d66532d49..f53f433b7c 100644 --- a/control_plane/src/bin/neon_local.rs +++ b/control_plane/src/bin/neon_local.rs @@ -640,6 +640,8 @@ async fn handle_timeline(timeline_match: &ArgMatches, env: &mut local_env::Local } Some(("branch", branch_match)) => { let tenant_id = get_tenant_id(branch_match, env)?; + let new_timeline_id = + parse_timeline_id(branch_match)?.unwrap_or(TimelineId::generate()); let new_branch_name = branch_match .get_one::("branch-name") .ok_or_else(|| anyhow!("No branch name provided"))?; @@ -658,7 +660,6 @@ async fn handle_timeline(timeline_match: &ArgMatches, env: &mut local_env::Local .map(|lsn_str| Lsn::from_str(lsn_str)) .transpose() .context("Failed to parse ancestor start Lsn from the request")?; - let new_timeline_id = TimelineId::generate(); let storage_controller = StorageController::from_env(env); let create_req = TimelineCreateRequest { new_timeline_id, @@ -1583,6 +1584,7 @@ fn cli() -> Command { .subcommand(Command::new("branch") .about("Create a new timeline, using another timeline as a base, copying its data") .arg(tenant_id_arg.clone()) + .arg(timeline_id_arg.clone()) .arg(branch_name_arg.clone()) .arg(Arg::new("ancestor-branch-name").long("ancestor-branch-name") .help("Use last Lsn of another timeline (and its data) as base when creating the new timeline. The timeline gets resolved by its branch name.").required(false)) diff --git a/test_runner/fixtures/neon_fixtures.py b/test_runner/fixtures/neon_fixtures.py index 5fba05c9e1..7386e25740 100644 --- a/test_runner/fixtures/neon_fixtures.py +++ b/test_runner/fixtures/neon_fixtures.py @@ -1523,13 +1523,6 @@ class PageserverPort: http: int -CREATE_TIMELINE_ID_EXTRACTOR: re.Pattern = re.compile( # type: ignore[type-arg] - r"^Created timeline '(?P[^']+)'", re.MULTILINE -) -TIMELINE_DATA_EXTRACTOR: re.Pattern = re.compile( # type: ignore[type-arg] - r"\s?(?P[^\s]+)\s\[(?P[^\]]+)\]", re.MULTILINE -) - class AbstractNeonCli(abc.ABC): """ @@ -1759,6 +1752,9 @@ class NeonCli(AbstractNeonCli): tenant_id: Optional[TenantId] = None, timeline_id: Optional[TimelineId] = None, ) -> TimelineId: + if timeline_id is None: + timeline_id = TimelineId.generate() + cmd = [ "timeline", "create", @@ -1766,23 +1762,16 @@ class NeonCli(AbstractNeonCli): new_branch_name, "--tenant-id", str(tenant_id or self.env.initial_tenant), + "--timeline-id", + str(timeline_id), "--pg-version", self.env.pg_version, ] - if timeline_id is not None: - cmd.extend(["--timeline-id", str(timeline_id)]) - res = self.raw_cli(cmd) res.check_returncode() - matches = CREATE_TIMELINE_ID_EXTRACTOR.search(res.stdout) - - created_timeline_id = None - if matches is not None: - created_timeline_id = matches.group("timeline_id") - - return TimelineId(str(created_timeline_id)) + return timeline_id def create_branch( self, @@ -1790,12 +1779,17 @@ class NeonCli(AbstractNeonCli): ancestor_branch_name: Optional[str] = None, tenant_id: Optional[TenantId] = None, ancestor_start_lsn: Optional[Lsn] = None, + new_timeline_id: Optional[TimelineId] = None, ) -> TimelineId: + if new_timeline_id is None: + new_timeline_id = TimelineId.generate() cmd = [ "timeline", "branch", "--branch-name", new_branch_name, + "--timeline-id", + str(new_timeline_id), "--tenant-id", str(tenant_id or self.env.initial_tenant), ] @@ -1807,16 +1801,7 @@ class NeonCli(AbstractNeonCli): res = self.raw_cli(cmd) res.check_returncode() - matches = CREATE_TIMELINE_ID_EXTRACTOR.search(res.stdout) - - created_timeline_id = None - if matches is not None: - created_timeline_id = matches.group("timeline_id") - - if created_timeline_id is None: - raise Exception("could not find timeline id after `neon timeline create` invocation") - else: - return TimelineId(str(created_timeline_id)) + return TimelineId(str(new_timeline_id)) def list_timelines(self, tenant_id: Optional[TenantId] = None) -> List[Tuple[str, TimelineId]]: """ @@ -1825,6 +1810,9 @@ class NeonCli(AbstractNeonCli): # main [b49f7954224a0ad25cc0013ea107b54b] # ┣━ @0/16B5A50: test_cli_branch_list_main [20f98c79111b9015d84452258b7d5540] + TIMELINE_DATA_EXTRACTOR: re.Pattern = re.compile( # type: ignore[type-arg] + r"\s?(?P[^\s]+)\s\[(?P[^\]]+)\]", re.MULTILINE + ) res = self.raw_cli( ["timeline", "list", "--tenant-id", str(tenant_id or self.env.initial_tenant)] )