Skip to content

Commit

Permalink
changes based on comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@salonig23
salonig23 committed May 10, 2024
1 parent 01210e0 commit da6a026
Show file tree
Hide file tree
Showing 7 changed files with 331 additions and 118 deletions.
4 changes: 3 additions & 1 deletion master/internal/api_trials.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ func (a *apiServer) GetTrialRemainingLogRetentionDays(
q := `
SELECT
CASE
WHEN MIN(t.end_time) <= ( retention_timestamp() - make_interval(days => ?) ) THEN 0
WHEN MIN(t.end_time) <= ( NOW() - make_interval(days => ?) ) THEN 0
ELSE extract(day from MIN(end_time) + make_interval(days => ?) - NOW())::int
END remaining_log_retention_days
FROM tasks t
Expand Down Expand Up @@ -1568,6 +1568,8 @@ func (a *apiServer) PostTrialRunnerMetadata(
func (a *apiServer) isTrialTerminalFunc(trialID int, buffer time.Duration) api.TerminationCheckFn {
return func() (bool, error) {
state, endTime, err := a.m.db.TrialStatus(trialID)
log.Print("state, ", state)
log.Print("endTime", endTime)
if err != nil ||
(model.TerminalStates[state] && endTime.Add(buffer).Before(time.Now().UTC())) {
return true, err
Expand Down
25 changes: 19 additions & 6 deletions master/internal/api_trials_intg_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1450,18 +1450,29 @@ func TestPutTrialRetainLogs(t *testing.T) {
}

func completeTrialsandTasks(ctx context.Context, trialID int, endTimeDays int) error {
_, err := db.Bun().NewUpdate().Table("runs").
_, err := db.Bun().NewUpdate().
Table("tasks", "run_id_task_id").
Set("end_time = (NOW() - make_interval(days => ?))", endTimeDays).
Where("run_id_task_id.run_id = ? and tasks.task_id = run_id_task_id.task_id", trialID).
Exec(ctx)
if err != nil {
return err
}
_, err = db.Bun().NewUpdate().Table("runs", "run_id_task_id", "tasks").
Set("state = ?", model.CompletedState).
Set("end_time = (NOW() - make_interval(days => ?))", endTimeDays).
Where("id = ?", trialID).
Exec(ctx)
if err != nil {
return err
}
return nil
}

_, err = db.Bun().NewUpdate().
Table("tasks", "run_id_task_id").
Set("end_time = (NOW() - make_interval(days => ?))", endTimeDays).
Where("run_id_task_id.run_id = ? and tasks.task_id = run_id_task_id.task_id", trialID).
func completeExp(ctx context.Context, expID int32) error {
_, err := db.Bun().NewUpdate().Table("experiments").
Set("state = ?", model.CompletedState).
Where("id = ?", expID).
Exec(ctx)
if err != nil {
return err
Expand Down Expand Up @@ -1491,7 +1502,7 @@ retention_policy:
for _, tt := range tests {
log.Printf("Starting %v", tt.name)
testEndTimes := []int{15, 10, 5, 0}
_, trialIDs, _ := CreateTestRetentionExperiment(ctx, t, api, tt.logRetentionDays, len(testEndTimes))
exp, trialIDs, _ := CreateTestRetentionExperiment(ctx, t, api, tt.logRetentionDays, len(testEndTimes))

for i, v := range testEndTimes {
err := completeTrialsandTasks(ctx, trialIDs[i], v)
Expand All @@ -1503,5 +1514,7 @@ retention_policy:
require.NoError(t, err)
require.Equal(t, tt.expRemainingDays[i], *d.RemainingDays)
}
err := completeExp(ctx, exp.Id)
require.NoError(t, err)
}
}
37 changes: 37 additions & 0 deletions webui/react/src/components/RemainingRetentionDaysLabelComponent.tsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Loadable } from 'hew/utils/loadable';
import React from 'react';

import { pluralizer } from 'utils/string';

import Badge from './Badge';

interface Props {
remainingLogDays: Loadable<number | undefined>;
}

const RemainingRetentionDaysLabelComponent: React.FC<Props> = ({ remainingLogDays }: Props) => {
let toolTipText = '';
let badgeText = '';
const days = Loadable.getOrElse(undefined, remainingLogDays);
if (days === undefined) {
toolTipText = 'Days remaining to retention are not available yet.';
badgeText = '-';
} else if (days === -1) {
toolTipText = 'Logs will be retained forever.';
badgeText = 'Frvr';
} else if (days === 0) {
toolTipText = 'Some logs have begun to be deleted for this trial.';
badgeText = '0';
} else {
toolTipText = `${days} ${pluralizer(days, 'day')} left to retain logs`;
badgeText = `${days}`;
}
return (
<div>
<span>Logs </span>
<Badge tooltip={toolTipText}>{badgeText}</Badge>
</div>
);
};

export default RemainingRetentionDaysLabelComponent;
70 changes: 15 additions & 55 deletions webui/react/src/pages/ExperimentDetails/ExperimentSingleTrialTabs.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { string } from 'io-ts';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { unstable_useBlocker, useLocation, useNavigate, useParams } from 'react-router-dom';

import Badge from 'components/Badge';
import HyperparameterSearchModalComponent from 'components/HyperparameterSearchModal';
import RemainingRetentionDaysLabelComponent from 'components/RemainingRetentionDaysLabelComponent';
import TrialLogPreview from 'components/TrialLogPreview';
import { UNMANAGED_MESSAGE } from 'constant';
import { terminalRunStates } from 'constants/states';
Expand All @@ -33,7 +33,6 @@ import {
} from 'services/api';
import { ExperimentBase, Note, TrialDetails, TrialItem, ValueOf } from 'types';
import handleError, { ErrorLevel, ErrorType } from 'utils/error';
import { pluralizer } from 'utils/string';

import ExperimentCheckpoints from './ExperimentCheckpoints';
import ExperimentCodeViewer from './ExperimentCodeViewer';
Expand Down Expand Up @@ -84,7 +83,7 @@ const ExperimentSingleTrialTabs: React.FC<Props> = ({
const [canceler] = useState(new AbortController());
const [trialDetails, setTrialDetails] = useState<TrialDetails>();
const [tabKey, setTabKey] = useState(tab && TAB_KEYS.includes(tab) ? tab : DEFAULT_TAB_KEY);
const [remainingLogDays, setRemainingLogDays] = useState<Loadable<number>>(NotLoaded);
const [remainingLogDays, setRemainingLogDays] = useState<Loadable<number | undefined>>(NotLoaded);

const waitingForTrials = !trialId && !wontHaveTrials;

Expand Down Expand Up @@ -139,12 +138,16 @@ const ExperimentSingleTrialTabs: React.FC<Props> = ({
}
}, [canceler, experiment.id, experiment.state, onTrialUpdate]);

const fetchTrialDetails = useCallback(async () => {
const fetchTrialData = useCallback(async () => {
if (!trialId) return;
try {
const response = await getTrialDetails({ id: trialId }, { signal: canceler.signal });
onTrialUpdate?.(response);
setTrialDetails(response);
const [trialDetailResponse, logRemainingResponse] = await Promise.all([
getTrialDetails({ id: trialId }, { signal: canceler.signal }),
getTrialRemainingLogRetentionDays({ id: trialId }),
]);
onTrialUpdate?.(trialDetailResponse);
setTrialDetails(trialDetailResponse);
setRemainingLogDays(Loaded(logRemainingResponse.remainingLogRetentionDays));
} catch (e) {
handleError(e, {
level: ErrorLevel.Error,
Expand All @@ -155,50 +158,7 @@ const ExperimentSingleTrialTabs: React.FC<Props> = ({
}
}, [canceler.signal, onTrialUpdate, trialId]);

const fetchRemainingLogDays = useCallback(async () => {
if (!trialId) return;
try {
const remainingDays = await getTrialRemainingLogRetentionDays({ id: trialId });
setRemainingLogDays(
remainingDays.remainingLogRetentionDays !== undefined
? Loaded(remainingDays.remainingLogRetentionDays)
: NotLoaded,
);
} catch (e) {
setRemainingLogDays(NotLoaded);
}
}, [trialId]);

const fetchAllTrialDetails = useCallback(async () => {
await Promise.allSettled([fetchTrialDetails(), fetchRemainingLogDays()]);
}, [fetchRemainingLogDays, fetchTrialDetails]);

const getRemainingLogsLabel = useMemo(() => {
let toolTipText = '';
let badgeText = '';
const days = Loadable.getOrElse(null, remainingLogDays);
if (days === null) {
toolTipText = 'Days remaining to retention are not available yet.';
badgeText = '-';
} else if (days === -1) {
toolTipText = 'Logs will be retained forever.';
badgeText = 'Frvr';
} else if (days === 0) {
toolTipText = 'Some logs have begun to be deleted for this trial.';
badgeText = '0';
} else {
toolTipText = `${pluralizer(days, 'day')} left to retain logs`;
badgeText = `${remainingLogDays}`;
}
return (
<div>
<>Logs </>
<Badge tooltip={toolTipText}>{badgeText}</Badge>
</div>
);
}, [remainingLogDays]);

const { stopPolling } = usePolling(fetchAllTrialDetails, { rerunOnNewFn: true });
const { stopPolling } = usePolling(fetchTrialData, { rerunOnNewFn: true });
const { stopPolling: stopPollingFirstTrialId } = usePolling(fetchFirstTrialId, {
rerunOnNewFn: true,
});
Expand Down Expand Up @@ -250,8 +210,8 @@ const ExperimentSingleTrialTabs: React.FC<Props> = ({
* next polling cycle when trial Id goes from undefined to defined.
*/
useEffect(() => {
if (prevTrialId === undefined && prevTrialId !== trialId) fetchAllTrialDetails();
}, [fetchAllTrialDetails, prevTrialId, trialId]);
if (prevTrialId === undefined && prevTrialId !== trialId) fetchTrialData();
}, [fetchTrialData, prevTrialId, trialId]);

const handleNotesUpdate = useCallback(
async (notes: Note) => {
Expand Down Expand Up @@ -371,18 +331,18 @@ const ExperimentSingleTrialTabs: React.FC<Props> = ({
<TrialDetailsLogs experiment={experiment} trial={trialDetails as TrialDetails} />
),
key: TabType.Logs,
label: getRemainingLogsLabel,
label: RemainingRetentionDaysLabelComponent({ remainingLogDays }),
});
}

return items;
}, [
editableNotes,
experiment,
getRemainingLogsLabel,
handleNotesUpdate,
handleSelectFile,
pageRef,
remainingLogDays,
settings.filePath,
showExperimentArtifacts,
trialDetails,
Expand Down
69 changes: 15 additions & 54 deletions webui/react/src/pages/TrialDetails.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { Loadable, Loaded, NotLoaded } from 'hew/utils/loadable';
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { useNavigate, useParams } from 'react-router-dom';

import Badge from 'components/Badge';
import Page from 'components/Page';
import RemainingRetentionDaysLabelComponent from 'components/RemainingRetentionDaysLabelComponent';
import RoutePagination from 'components/RoutePagination';
import TrialLogPreview from 'components/TrialLogPreview';
import { terminalRunStates } from 'constants/states';
Expand All @@ -30,7 +30,6 @@ import handleError, { ErrorType } from 'utils/error';
import { isSingleTrialExperiment } from 'utils/experiment';
import { useObservable } from 'utils/observable';
import { isAborted, isNotFound } from 'utils/service';
import { pluralizer } from 'utils/string';

import MultiTrialDetailsHyperparameters from './TrialDetails/MultiTrialDetailsHyperparameters';

Expand Down Expand Up @@ -71,7 +70,7 @@ const TrialDetailsComp: React.FC = () => {
const workspaces = Loadable.getOrElse([], useObservable(workspaceStore.workspaces));
const basePath = paths.trialDetails(trialId, experimentId);
const trial = trialDetails.data;
const [remainingLogDays, setRemainingLogDays] = useState<Loadable<number>>(NotLoaded);
const [remainingLogDays, setRemainingLogDays] = useState<Loadable<number | undefined>>(NotLoaded);

const showExperimentArtifacts = usePermissions().canViewExperimentArtifacts({
workspace: { id: experiment?.workspaceId ?? 0 },
Expand Down Expand Up @@ -104,59 +103,21 @@ const TrialDetailsComp: React.FC = () => {
}
}, [canceler, navigate, experimentId, trial]);

const fetchTrialDetails = useCallback(async () => {
const fetchTrialData = useCallback(async () => {
try {
const response = await getTrialDetails({ id: trialId }, { signal: canceler.signal });
setTrialDetails((prev) => ({ ...prev, data: response }));
const [trialDetailResponse, logRemainingResponse] = await Promise.all([
getTrialDetails({ id: trialId }, { signal: canceler.signal }),
getTrialRemainingLogRetentionDays({ id: trialId }),
]);
setTrialDetails((prev) => ({ ...prev, data: trialDetailResponse }));
setRemainingLogDays(Loaded(logRemainingResponse.remainingLogRetentionDays));
} catch (e) {
if (!trialDetails.error && !isAborted(e)) {
setTrialDetails((prev) => ({ ...prev, error: e as Error }));
}
}
}, [canceler, trialDetails.error, trialId]);

const fetchRemainingLogDays = useCallback(async () => {
try {
const remainingDays = await getTrialRemainingLogRetentionDays({ id: trialId });
setRemainingLogDays(
remainingDays.remainingLogRetentionDays !== undefined
? Loaded(remainingDays.remainingLogRetentionDays)
: NotLoaded,
);
} catch (e) {
setRemainingLogDays(NotLoaded);
}
}, [trialId]);

const fetchAllTrialDetails = useCallback(async () => {
await Promise.allSettled([fetchTrialDetails(), fetchRemainingLogDays()]);
}, [fetchRemainingLogDays, fetchTrialDetails]);

const getRemainingLogsLabel = useMemo(() => {
let toolTipText = '';
let badgeText = '';
const days = Loadable.getOrElse(null, remainingLogDays);
if (days === null) {
toolTipText = 'Days remaining to retention are not available yet.';
badgeText = '-';
} else if (days === -1) {
toolTipText = 'Logs will be retained forever.';
badgeText = 'Frvr';
} else if (days === 0) {
toolTipText = 'Some logs have begun to be deleted for this trial.';
badgeText = '0';
} else {
toolTipText = `${pluralizer(days, 'day')} left to retain logs`;
badgeText = `${remainingLogDays}`;
}
return (
<div>
<>Logs </>
<Badge tooltip={toolTipText}>{badgeText}</Badge>
</div>
);
}, [remainingLogDays]);

const handleTabChange = useCallback(
(key: string) => {
setTabKey(key as TabType);
Expand Down Expand Up @@ -210,7 +171,7 @@ const TrialDetailsComp: React.FC = () => {
{
children: <TrialDetailsLogs experiment={experiment} trial={trial} />,
key: TabType.Logs,
label: getRemainingLogsLabel,
label: RemainingRetentionDaysLabelComponent({ remainingLogDays }),
},
];

Expand All @@ -223,18 +184,18 @@ const TrialDetailsComp: React.FC = () => {
}

return tabs;
}, [experiment, trial, getRemainingLogsLabel, showExperimentArtifacts]);
}, [experiment, trial, remainingLogDays, showExperimentArtifacts]);

const { stopPolling } = usePolling(fetchAllTrialDetails);
const { stopPolling } = usePolling(fetchTrialData);

useEffect(() => {
setTrialId(Number(trialID));
}, [trialID]);

useEffect(() => {
setIsFetching(true);
fetchAllTrialDetails();
}, [fetchAllTrialDetails, trialId]);
fetchTrialData();
}, [fetchTrialData, trialId]);

useEffect(() => {
fetchExperimentDetails();
Expand Down Expand Up @@ -290,7 +251,7 @@ const TrialDetailsComp: React.FC = () => {
headerComponent={
<TrialDetailsHeader
experiment={experiment}
fetchTrialDetails={fetchAllTrialDetails}
fetchTrialDetails={fetchTrialData}
trial={trial}
/>
}
Expand Down

0 comments on commit da6a026

Please sign in to comment.