Skip to content

Commit 7aa2173

Browse files
Merge pull request #375 from microsoft/psl-fix-gp-bug
fix: gp bug and Managed identity changes
2 parents 22932f1 + f518de4 commit 7aa2173

2 files changed

Lines changed: 26 additions & 90 deletions

File tree

src/backend/sql_agents/convert_script.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ async def convert_script(
6565

6666
# orchestrate the chat
6767
current_migration = "No migration"
68-
while True:
68+
is_complete: bool = False
69+
while not is_complete:
6970
await comms_manager.group_chat.add_chat_message(
7071
ChatMessageContent(role=AuthorRole.USER, content=source_script)
7172
)
@@ -99,6 +100,7 @@ async def convert_script(
99100
AuthorRole(response.role),
100101
)
101102
current_migration = None
103+
is_complete = True
102104
break
103105
case AgentType.SYNTAX_CHECKER.value:
104106
result = SyntaxCheckerResponse.model_validate_json(
@@ -269,10 +271,11 @@ async def convert_script(
269271
FileResult.ERROR,
270272
),
271273
)
274+
is_complete = True
272275
break
273276

274277
if comms_manager.group_chat.is_complete:
275-
break
278+
is_complete = True
276279

277280
migrated_query = current_migration
278281

src/frontend/src/pages/modernizationPage.tsx

Lines changed: 21 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -497,10 +497,11 @@ const ModernizationPage = () => {
497497
const [fileId, setFileId] = React.useState<string>("");
498498
const [expandedSections, setExpandedSections] = React.useState<string[]>([]);
499499
const [allFilesCompleted, setAllFilesCompleted] = useState(false);
500+
const [progressPercentage, setProgressPercentage] = useState(0);
500501
const [isZipButtonDisabled, setIsZipButtonDisabled] = useState(true);
501502
const [fileLoading, setFileLoading] = useState(false);
502503
const [lastActivityTime, setLastActivityTime] = useState<number>(Date.now());
503-
const [pageLoadTime] = useState<number>(Date.now());
504+
//const [pageLoadTime] = useState<number>(Date.now());
504505

505506
// Fetch file content when a file is selected
506507
useEffect(() => {
@@ -514,18 +515,9 @@ const ModernizationPage = () => {
514515
if (!selectedFile || !selectedFile.translatedCode) {
515516
setFileLoading(true);
516517
const newFileUpdate = await fetchFileFromAPI(selectedFile?.fileId || "");
517-
setFiles((prevFiles) =>
518-
prevFiles.map((file) =>
519-
file.fileId === selectedFile?.fileId
520-
? {
521-
...file,
522-
code: newFileUpdate.content,
523-
translatedCode: newFileUpdate.translated_content,
524-
}
525-
: file
526-
)
527-
);
528518
setFileLoading(false);
519+
} else {
520+
529521
}
530522

531523
} catch (err) {
@@ -603,18 +595,9 @@ const ModernizationPage = () => {
603595
fetchBatchData(batchId);
604596
}, [batchId]);
605597

606-
// Listen for startProcessing completion and navigate to batch view
607-
useEffect(() => {
608-
if (batchState && !batchState.loading && batchState.status === "Processing completed") {
609-
console.log("Start processing API completed successfully - processing is done!");
610-
611-
// Check if we have the response with batch_id that matches current batchId
612-
if (batchState.batchId === batchId) {
613-
console.log("Processing completed for current batch, navigating to batch view page");
614-
navigate(`/batch-view/${batchId}`);
615-
}
616-
}
617-
}, [batchState.loading, batchState.status, batchState.batchId, batchId, navigate]);
598+
// Do NOT navigate based on Redux startProcessing state.
599+
// The start-processing API may return 504 even if backend work is ongoing.
600+
// Navigation is ONLY triggered by actual file completion via WebSocket/polling.
618601

619602
const handleDownloadZip = async () => {
620603
if (batchId) {
@@ -811,37 +794,17 @@ const ModernizationPage = () => {
811794
const latestBatch = await fetchBatchSummary(batchId!);
812795
setBatchSummary(latestBatch);
813796

814-
// Check if all files are in terminal states OR if the batch itself is marked as completed
797+
// Only complete when all files reach terminal states.
815798
const allFilesDone = latestBatch.files.every(file =>
816799
["completed", "failed", "error"].includes(file.status?.toLowerCase() || "")
817800
);
818-
819-
// Also check if batch status indicates completion (for cases where some files remain queued)
820-
const batchCompleted = latestBatch.status?.toLowerCase() === "completed" ||
821-
latestBatch.status?.toLowerCase() === "failed";
822-
823-
// Special handling for stuck processing files - if no completed files and long time passed
824-
const hasProcessingFiles = latestBatch.files.some(file =>
825-
file.status?.toLowerCase() === "in_process"
826-
);
827-
const hasCompletedFiles = latestBatch.files.some(file =>
828-
file.status?.toLowerCase() === "completed"
829-
);
830-
const timeSinceLastActivity = Date.now() - lastActivityTime;
831-
const likelyStuckProcessing = hasProcessingFiles &&
832-
!hasCompletedFiles &&
833-
timeSinceLastActivity > 60000; // 60 seconds of no activity
834-
835-
// Consider processing done if either all files are terminal OR batch is marked complete OR files appear stuck
836-
const processingComplete = allFilesDone || batchCompleted || likelyStuckProcessing;
801+
802+
const processingComplete = allFilesDone;
837803

838804
if (processingComplete) {
839805
console.log("Processing complete detected:", {
840806
allFilesDone,
841-
batchCompleted,
842-
likelyStuckProcessing,
843-
batchStatus: latestBatch.status,
844-
timeSinceActivity: timeSinceLastActivity
807+
batchStatus: latestBatch.status
845808
});
846809
setAllFilesCompleted(true);
847810
const hasUsableFile = latestBatch.files.some(file =>
@@ -868,8 +831,8 @@ const ModernizationPage = () => {
868831
return updated;
869832
});
870833

871-
// Navigate to batch view page when processing is complete
872-
console.log("Processing complete (either all files done or batch completed), navigating to batch view page");
834+
// Navigate only after all files have reached terminal states.
835+
console.log("Processing complete (all files done), navigating to batch view page");
873836
navigate(`/batch-view/${batchId}`);
874837
}
875838
} catch (err) {
@@ -950,17 +913,8 @@ useEffect(() => {
950913
file.id === "summary" || // skip summary
951914
["completed", "failed", "error"].includes(file.status?.toLowerCase() || "")
952915
);
953-
954-
// Also check if we have at least one completed file and no files currently processing
955-
const hasCompletedFiles = files.some(file =>
956-
file.id !== "summary" && file.status === "completed"
957-
);
958-
const hasProcessingFiles = files.some(file =>
959-
file.id !== "summary" && file.status === "in_process"
960-
);
961-
962-
// Consider done if all terminal OR (has completed files and no processing files)
963-
const effectivelyDone = areAllFilesTerminal || (hasCompletedFiles && !hasProcessingFiles);
916+
917+
const effectivelyDone = areAllFilesTerminal;
964918

965919
if (files.length > 1 && effectivelyDone && !allFilesCompleted) {
966920
console.log("Files processing appears complete, checking batch status");
@@ -1013,55 +967,34 @@ useEffect(() => {
1013967
};
1014968
}, [handleWebSocketMessage]);
1015969

1016-
// Set a timeout for initial loading - if still loading after 30 seconds, show a warning message
970+
// Set a timeout for initial loading - if no progress after 30 seconds, show error
1017971
useEffect(() => {
1018972
const loadingTimeout = setTimeout(() => {
1019-
if (showLoading) {
973+
if (progressPercentage < 5 && showLoading) {
1020974
setLoadingError('Processing is taking longer than expected. You can continue waiting or try again later.');
1021975
}
1022976
}, 30000);
1023977

1024978
return () => clearTimeout(loadingTimeout);
1025-
}, [showLoading]);
979+
}, [progressPercentage, showLoading]);
1026980

1027-
// Add timeout mechanism to navigate if no activity for 30 seconds
981+
// Poll summary status during inactivity, but do not force completion/navigation by timeout.
1028982
useEffect(() => {
1029983
const checkInactivity = setInterval(() => {
1030984
const timeSinceLastActivity = Date.now() - lastActivityTime;
1031985
const hasCompletedFiles = files.some(file =>
1032986
file.id !== "summary" && file.status === "completed"
1033987
);
1034-
const hasProcessingFiles = files.some(file =>
1035-
file.id !== "summary" && file.status === "in_process"
1036-
);
1037-
const nonSummaryFiles = files.filter(f => f.id !== "summary");
1038988

1039-
// If we have completed files and no activity for 30 seconds, check if we should navigate
989+
// If we have completed files and no activity for 30 seconds, refresh status.
1040990
if (hasCompletedFiles && timeSinceLastActivity > 30000 && !allFilesCompleted) {
1041991
console.log("No activity for 30 seconds with completed files, checking final status");
1042992
updateSummaryStatus();
1043993
}
1044-
1045-
// Special case: If only harmful files that are stuck in processing for 60+ seconds
1046-
if (nonSummaryFiles.length > 0 &&
1047-
hasProcessingFiles &&
1048-
!hasCompletedFiles &&
1049-
timeSinceLastActivity > 60000 &&
1050-
!allFilesCompleted) {
1051-
console.log("Files stuck in processing for 60+ seconds, likely failed - checking batch status");
1052-
updateSummaryStatus();
1053-
}
1054-
1055-
// Ultimate fallback: If on page for 2+ minutes with no completion, force navigation
1056-
const timeSincePageLoad = Date.now() - pageLoadTime;
1057-
if (timeSincePageLoad > 120000 && !allFilesCompleted && nonSummaryFiles.length > 0) {
1058-
console.log("Page loaded for 2+ minutes without completion, forcing navigation to batch view");
1059-
navigate(`/batch-view/${batchId}`);
1060-
}
1061994
}, 5000); // Check every 5 seconds
1062995

1063996
return () => clearInterval(checkInactivity);
1064-
}, [lastActivityTime, files, allFilesCompleted, updateSummaryStatus, pageLoadTime, navigate, batchId]);
997+
}, [lastActivityTime, files, allFilesCompleted, updateSummaryStatus, navigate, batchId]);
1065998

1066999

10671000
useEffect(() => {

0 commit comments

Comments
 (0)