Skip to content

Commit 2c53dbd

Browse files
brianajbrianaj
authored
Merge pull request #1455 from github/brianaj-use-migration-id-for-download-logs
Add support for downloading logs by migration ID
2 parents 381a83f + f68edcb commit 2c53dbd

File tree

9 files changed

+199
-26
lines changed

9 files changed

+199
-26
lines changed

‎RELEASENOTES.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1+
- Added `--migration-id` option to `download-logs` command to allow downloading logs directly by migration ID without requiring org/repo lookup

‎src/Octoshift/Commands/DownloadLogs/DownloadLogsCommandArgs.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ public class DownloadLogsCommandArgs : CommandArgs
55
{
66
public string GithubOrg { get; set; }
77
public string GithubRepo { get; set; }
8+
public string MigrationId { get; set; }
89
public string GithubApiUrl { get; set; }
910
[Secret]
1011
public string GithubPat { get; set; }

‎src/Octoshift/Commands/DownloadLogs/DownloadLogsCommandBase.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,19 @@ public class DownloadLogsCommandBase : CommandBase<DownloadLogsCommandArgs, Down
1818

1919
public virtual Option<string> GithubOrg { get; } = new("--github-org")
2020
{
21-
IsRequired = true,
2221
Description = "GitHub organization to download logs from."
2322
};
2423

2524
public virtual Option<string> GithubRepo { get; } = new("--github-repo")
2625
{
27-
IsRequired = true,
2826
Description = "Target repository to download latest log for."
2927
};
3028

29+
public virtual Option<string> MigrationId { get; } = new("--migration-id")
30+
{
31+
Description = "Migration ID to download logs for. If specified, --github-org and --github-repo are not required."
32+
};
33+
3134
public virtual Option<string> GithubApiUrl { get; } = new("--github-api-url")
3235
{
3336
Description = "Target GitHub API URL if not targeting github.com (default: https://api.github.com)."
@@ -79,6 +82,7 @@ protected void AddOptions()
7982
{
8083
AddOption(GithubOrg);
8184
AddOption(GithubRepo);
85+
AddOption(MigrationId);
8286
AddOption(GithubApiUrl);
8387
AddOption(GithubPat);
8488
AddOption(MigrationLogFile);

‎src/Octoshift/Commands/DownloadLogs/DownloadLogsCommandHandler.cs‎

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,21 @@ public async Task Handle(DownloadLogsCommandArgs args)
3838
throw new ArgumentNullException(nameof(args));
3939
}
4040

41+
if (args.MigrationId.HasValue())
42+
{
43+
if (args.GithubOrg.HasValue() || args.GithubRepo.HasValue())
44+
{
45+
_log.LogWarning("--github-org and --github-repo are ignored when --migration-id is specified.");
46+
}
47+
}
48+
else
49+
{
50+
if (!args.GithubOrg.HasValue() || !args.GithubRepo.HasValue())
51+
{
52+
throw new OctoshiftCliException("Either --migration-id (GraphQL migration ID) or both --github-org and --github-repo must be specified.");
53+
}
54+
}
55+
4156
_log.LogWarning("Migration logs are only available for 24 hours after a migration finishes!");
4257

4358
_log.LogInformation("Downloading migration logs...");
@@ -49,22 +64,52 @@ public async Task Handle(DownloadLogsCommandArgs args)
4964
throw new OctoshiftCliException($"File {args.MigrationLogFile} already exists! Use --overwrite to overwrite this file.");
5065
}
5166

52-
var result = await _retryPolicy.RetryOnResult(async () => await _githubApi.GetMigrationLogUrl(args.GithubOrg, args.GithubRepo), r => r?.MigrationLogUrl.IsNullOrWhiteSpace() ?? false,
53-
"Waiting for migration log to populate...");
67+
string logUrl;
68+
string migrationId;
69+
string repositoryName;
5470

55-
if (result.Outcome == OutcomeType.Successful && result.Result is null)
71+
if (args.MigrationId.HasValue())
5672
{
57-
throw new OctoshiftCliException($"Migration for repository {args.GithubRepo} not found!");
58-
}
73+
// Use migration ID directly
74+
migrationId = args.MigrationId;
75+
var migrationResult = await _retryPolicy.RetryOnResult(
76+
async () => await _githubApi.GetMigration(migrationId),
77+
r => string.IsNullOrWhiteSpace(r.MigrationLogUrl),
78+
"Waiting for migration log to populate...");
79+
80+
if (migrationResult.Outcome == OutcomeType.Failure)
81+
{
82+
throw new OctoshiftCliException($"Migration log for migration {migrationId} is currently unavailable. Migration logs are only available for 24 hours after a migration finishes. Please ensure the migration ID is correct and the migration has completed recently.");
83+
}
5984

60-
if (result.Outcome == OutcomeType.Failure)
61-
{
62-
throw new OctoshiftCliException($"Migration log for repository {args.GithubRepo} unavailable!");
85+
var (_, RepositoryName, _, _, MigrationLogUrl) = migrationResult.Result;
86+
logUrl = MigrationLogUrl;
87+
repositoryName = RepositoryName;
6388
}
89+
else
90+
{
91+
// Use org/repo to find migration
92+
var result = await _retryPolicy.RetryOnResult(async () => await _githubApi.GetMigrationLogUrl(args.GithubOrg, args.GithubRepo), r => r?.MigrationLogUrl.IsNullOrWhiteSpace() ?? false,
93+
"Waiting for migration log to populate...");
94+
95+
if (result.Outcome == OutcomeType.Successful && result.Result is null)
96+
{
97+
throw new OctoshiftCliException($"Migration for repository {args.GithubRepo} not found!");
98+
}
6499

65-
var (logUrl, migrationId) = result.Result.Value;
100+
if (result.Outcome == OutcomeType.Failure)
101+
{
102+
throw new OctoshiftCliException($"Migration log for repository {args.GithubRepo} unavailable!");
103+
}
104+
105+
(logUrl, migrationId) = result.Result.Value;
106+
repositoryName = args.GithubRepo;
107+
}
66108

67-
args.MigrationLogFile ??= $"migration-log-{args.GithubOrg}-{args.GithubRepo}-{migrationId}.log";
109+
var defaultFileName = args.MigrationId.HasValue()
110+
? $"migration-log-{repositoryName}-{migrationId}.log"
111+
: $"migration-log-{args.GithubOrg}-{repositoryName}-{migrationId}.log";
112+
args.MigrationLogFile ??= defaultFileName;
68113

69114
if (FileExists(args.MigrationLogFile))
70115
{
@@ -76,9 +121,9 @@ public async Task Handle(DownloadLogsCommandArgs args)
76121
_log.LogWarning($"Overwriting {args.MigrationLogFile} due to --overwrite option.");
77122
}
78123

79-
_log.LogInformation($"Downloading log for repository {args.GithubRepo} to {args.MigrationLogFile}...");
124+
_log.LogInformation($"Downloading log for repository {repositoryName} to {args.MigrationLogFile}...");
80125
await _httpDownloadService.DownloadToFile(logUrl, args.MigrationLogFile);
81126

82-
_log.LogSuccess($"Downloaded {args.GithubRepo} log to {args.MigrationLogFile}.");
127+
_log.LogSuccess($"Downloaded {repositoryName} log to {args.MigrationLogFile}.");
83128
}
84129
}

‎src/OctoshiftCLI.Tests/Octoshift/Commands/DownloadLogs/DownloadLogsCommandHandlerTests.cs‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,4 +299,126 @@ await FluentActions
299299
_mockGithubApi.Verify(m => m.GetMigrationLogUrl(githubOrg, repo), Times.Exactly(6));
300300
_mockHttpDownloadService.Verify(m => m.DownloadToFile(It.IsAny<string>(), It.IsAny<string>()), Times.Never());
301301
}
302+
303+
[Fact]
304+
public async Task Should_Throw_When_Neither_MigrationId_Nor_OrgRepo_Provided()
305+
{
306+
// Act & Assert
307+
var args = new DownloadLogsCommandArgs();
308+
await FluentAssertions.FluentActions
309+
.Invoking(async () => await _handler.Handle(args))
310+
.Should().ThrowAsync<OctoshiftCliException>()
311+
.WithMessage("Either --migration-id (GraphQL migration ID) or both --github-org and --github-repo must be specified.");
312+
}
313+
314+
[Fact]
315+
public async Task Should_Throw_When_Only_GithubOrg_Provided()
316+
{
317+
// Act & Assert
318+
var args = new DownloadLogsCommandArgs
319+
{
320+
GithubOrg = "test-org"
321+
};
322+
await FluentAssertions.FluentActions
323+
.Invoking(async () => await _handler.Handle(args))
324+
.Should().ThrowAsync<OctoshiftCliException>()
325+
.WithMessage("Either --migration-id (GraphQL migration ID) or both --github-org and --github-repo must be specified.");
326+
}
327+
328+
[Fact]
329+
public async Task Should_Throw_When_Only_GithubRepo_Provided()
330+
{
331+
// Act & Assert
332+
var args = new DownloadLogsCommandArgs
333+
{
334+
GithubRepo = "test-repo"
335+
};
336+
await FluentAssertions.FluentActions
337+
.Invoking(async () => await _handler.Handle(args))
338+
.Should().ThrowAsync<OctoshiftCliException>()
339+
.WithMessage("Either --migration-id (GraphQL migration ID) or both --github-org and --github-repo must be specified.");
340+
}
341+
342+
[Fact]
343+
public async Task Should_Log_Warning_When_MigrationId_And_OrgRepo_Both_Provided()
344+
{
345+
// Arrange
346+
const string migrationId = "RM_test123";
347+
const string githubOrg = "test-org";
348+
const string githubRepo = "test-repo";
349+
const string logUrl = "some-url";
350+
const string repoName = "test-repo-name";
351+
352+
_mockGithubApi.Setup(m => m.GetMigration(migrationId))
353+
.ReturnsAsync((State: "SUCCEEDED", RepositoryName: repoName, WarningsCount: 0, FailureReason: "", MigrationLogUrl: logUrl));
354+
_mockHttpDownloadService.Setup(m => m.DownloadToFile(It.IsAny<string>(), It.IsAny<string>()));
355+
356+
// Act
357+
var args = new DownloadLogsCommandArgs
358+
{
359+
MigrationId = migrationId,
360+
GithubOrg = githubOrg,
361+
GithubRepo = githubRepo
362+
};
363+
await _handler.Handle(args);
364+
365+
// Assert
366+
_mockLogger.Verify(m => m.LogWarning("--github-org and --github-repo are ignored when --migration-id is specified."), Times.Once);
367+
_mockGithubApi.Verify(m => m.GetMigration(migrationId), Times.Once);
368+
_mockGithubApi.Verify(m => m.GetMigrationLogUrl(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
369+
}
370+
371+
[Fact]
372+
public async Task Should_Succeed_When_Only_MigrationId_Provided()
373+
{
374+
// Arrange
375+
const string migrationId = "RM_test123";
376+
const string logUrl = "some-url";
377+
const string repoName = "test-repo-name";
378+
const string expectedFileName = $"migration-log-{repoName}-{migrationId}.log";
379+
380+
_mockGithubApi.Setup(m => m.GetMigration(migrationId))
381+
.ReturnsAsync((State: "SUCCEEDED", RepositoryName: repoName, WarningsCount: 0, FailureReason: "", MigrationLogUrl: logUrl));
382+
_mockHttpDownloadService.Setup(m => m.DownloadToFile(It.IsAny<string>(), It.IsAny<string>()));
383+
384+
// Act
385+
var args = new DownloadLogsCommandArgs
386+
{
387+
MigrationId = migrationId
388+
};
389+
await _handler.Handle(args);
390+
391+
// Assert
392+
_mockGithubApi.Verify(m => m.GetMigration(migrationId), Times.Once);
393+
_mockHttpDownloadService.Verify(m => m.DownloadToFile(logUrl, expectedFileName), Times.Once);
394+
_mockGithubApi.Verify(m => m.GetMigrationLogUrl(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
395+
}
396+
397+
[Fact]
398+
public async Task Should_Succeed_When_Only_OrgRepo_Provided()
399+
{
400+
// Arrange
401+
const string githubOrg = "test-org";
402+
const string githubRepo = "test-repo";
403+
const string logUrl = "some-url";
404+
const string migrationId = "RM_test123";
405+
const string expectedFileName = $"migration-log-{githubOrg}-{githubRepo}-{migrationId}.log";
406+
407+
_mockGithubApi.Setup(m => m.GetMigrationLogUrl(githubOrg, githubRepo))
408+
.ReturnsAsync((logUrl, migrationId));
409+
_mockHttpDownloadService.Setup(m => m.DownloadToFile(It.IsAny<string>(), It.IsAny<string>()));
410+
411+
// Act
412+
var args = new DownloadLogsCommandArgs
413+
{
414+
GithubOrg = githubOrg,
415+
GithubRepo = githubRepo
416+
};
417+
await _handler.Handle(args);
418+
419+
// Assert
420+
_mockGithubApi.Verify(m => m.GetMigrationLogUrl(githubOrg, githubRepo), Times.Once);
421+
_mockHttpDownloadService.Verify(m => m.DownloadToFile(logUrl, expectedFileName), Times.Once);
422+
_mockGithubApi.Verify(m => m.GetMigration(It.IsAny<string>()), Times.Never);
423+
}
302424
}

‎src/OctoshiftCLI.Tests/ado2gh/Commands/DownloadLogs/DownloadLogsCommandTests.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ public void Should_Have_Options()
1111
var command = new DownloadLogsCommand();
1212
Assert.NotNull(command);
1313
Assert.Equal("download-logs", command.Name);
14-
Assert.Equal(7, command.Options.Count);
14+
Assert.Equal(8, command.Options.Count);
1515

16-
TestHelpers.VerifyCommandOption(command.Options, "github-org", true);
17-
TestHelpers.VerifyCommandOption(command.Options, "github-repo", true);
16+
TestHelpers.VerifyCommandOption(command.Options, "github-org", false);
17+
TestHelpers.VerifyCommandOption(command.Options, "github-repo", false);
18+
TestHelpers.VerifyCommandOption(command.Options, "migration-id", false);
1819
TestHelpers.VerifyCommandOption(command.Options, "github-api-url", false);
1920
TestHelpers.VerifyCommandOption(command.Options, "github-pat", false);
2021
TestHelpers.VerifyCommandOption(command.Options, "migration-log-file", false);

‎src/OctoshiftCLI.Tests/bbs2gh/Commands/DownloadLogs/DownloadLogsCommandTests.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ public void Should_Have_Options()
1111
var command = new DownloadLogsCommand();
1212
Assert.NotNull(command);
1313
Assert.Equal("download-logs", command.Name);
14-
Assert.Equal(7, command.Options.Count);
14+
Assert.Equal(8, command.Options.Count);
1515

16-
TestHelpers.VerifyCommandOption(command.Options, "github-org", true);
17-
TestHelpers.VerifyCommandOption(command.Options, "github-repo", true);
16+
TestHelpers.VerifyCommandOption(command.Options, "github-org", false);
17+
TestHelpers.VerifyCommandOption(command.Options, "github-repo", false);
18+
TestHelpers.VerifyCommandOption(command.Options, "migration-id", false);
1819
TestHelpers.VerifyCommandOption(command.Options, "github-api-url", false);
1920
TestHelpers.VerifyCommandOption(command.Options, "github-pat", false);
2021
TestHelpers.VerifyCommandOption(command.Options, "migration-log-file", false);

‎src/OctoshiftCLI.Tests/gei/Commands/DownloadLogs/DownloadLogsCommandTests.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ public void Should_Have_Options()
1111
var command = new DownloadLogsCommand();
1212
Assert.NotNull(command);
1313
Assert.Equal("download-logs", command.Name);
14-
Assert.Equal(7, command.Options.Count);
14+
Assert.Equal(8, command.Options.Count);
1515

16-
TestHelpers.VerifyCommandOption(command.Options, "github-target-org", true);
17-
TestHelpers.VerifyCommandOption(command.Options, "target-repo", true);
16+
TestHelpers.VerifyCommandOption(command.Options, "github-target-org", false);
17+
TestHelpers.VerifyCommandOption(command.Options, "target-repo", false);
18+
TestHelpers.VerifyCommandOption(command.Options, "migration-id", false);
1819
TestHelpers.VerifyCommandOption(command.Options, "target-api-url", false);
1920
TestHelpers.VerifyCommandOption(command.Options, "github-target-pat", false);
2021
TestHelpers.VerifyCommandOption(command.Options, "migration-log-file", false);

‎src/gei/Commands/DownloadLogs/DownloadLogsCommand.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,11 @@ public class DownloadLogsCommand : DownloadLogsCommandBase
2222

2323
public override Option<string> GithubRepo { get; } = new("--target-repo")
2424
{
25-
IsRequired = true,
2625
Description = "Target repository to download latest log for."
2726
};
2827

2928
public override Option<string> GithubOrg { get; } = new("--github-target-org")
3029
{
31-
IsRequired = true,
3230
Description = "Target GitHub organization to download logs from."
3331
};
3432
}

0 commit comments

Comments
 (0)