Skip to content

Commit

Permalink
Merge branch 'main' into actually-making-client-update-works
Browse files Browse the repository at this point in the history
  • Loading branch information
lotif authored Dec 9, 2024
2 parents 3da12ea + a6d0cf3 commit 729ba67
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/docs_deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:
# attempt_delay: 30000
#
- name: Deploy to Github pages
uses: JamesIves/github-pages-deploy-action@v4.6.9
uses: JamesIves/github-pages-deploy-action@v4.7.2
with:
branch: github_pages
folder: docs/build/html
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ repos:
- id: check-toml

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: 'v0.8.0'
rev: 'v0.8.1'
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
Expand Down
5 changes: 5 additions & 0 deletions florist/app/assets/css/florist.css
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@
margin: 0;
}

.job-details-download-button a.btn {
padding: 0;
margin: 15px 0 0 0;
}

.job-round-details {
padding-left: 40px;
}
Expand Down
26 changes: 24 additions & 2 deletions florist/app/jobs/details/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,12 @@ export function JobProgressBar({
metrics,
totalEpochs,
status,
clientIndex,
}: {
metrics: string;
totalEpochs: number;
status: status;
clientIndex: number;
}): ReactElement {
const [collapsed, setCollapsed] = useState(true);

Expand Down Expand Up @@ -271,14 +273,16 @@ export function JobProgressBar({
</a>
</div>
</div>
<div className="row pb-2">{!collapsed ? <JobProgressDetails metrics={metricsJson} /> : null}</div>
<div className="row pb-2">
{!collapsed ? <JobProgressDetails metrics={metricsJson} clientIndex={clientIndex} /> : null}
</div>
</div>
</div>
</div>
);
}

export function JobProgressDetails({ metrics }: { metrics: Object }): ReactElement {
export function JobProgressDetails({ metrics, clientIndex }: { metrics: Object; clientIndex: number }): ReactElement {
if (!metrics) {
return null;
}
Expand Down Expand Up @@ -309,6 +313,9 @@ export function JobProgressDetails({ metrics }: { metrics: Object }): ReactEleme
}
}

let metricsFileName = metrics.host_type === "server" ? "server-metrics.json" : `client-metrics-${clientIndex}.json`;
let metricsFileURL = window.URL.createObjectURL(new Blob([JSON.stringify(metrics, null, 4)]));

return (
<div className="job-progress-detail">
<div className="row">
Expand Down Expand Up @@ -337,6 +344,20 @@ export function JobProgressDetails({ metrics }: { metrics: Object }): ReactEleme
{roundMetricsArray.map((roundMetrics, i) => (
<JobProgressRound roundMetrics={roundMetrics} key={i} index={i} />
))}

<div className="row">
<div className="col-sm-4 job-details-download-button">
<a
className="btn btn-link download-metrics-button"
title="Download metrics as JSON"
href={metricsFileURL}
download={metricsFileName}
>
<i className="material-icons">download</i>
Download Metrics as JSON
</a>
</div>
</div>
</div>
);
}
Expand Down Expand Up @@ -647,6 +668,7 @@ export function JobDetailsClientsInfoTable({
<JobProgressBar
metrics={clientInfo.metrics}
totalEpochs={properties.totalEpochs}
clientIndex={i}
/>
</span>
</div>
Expand Down
72 changes: 72 additions & 0 deletions florist/tests/unit/app/jobs/details/page.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ function setupGetJobMock(data: JobData, isLoading: boolean = false, error = null
});
}

function setupURLSpyMock(urlSpy, testURL: string = "foo") {
urlSpy = jest.spyOn(window, "URL");
urlSpy.createObjectURL = jest.fn((_) => testURL);
return urlSpy;
}

function makeTestJob(): JobData {
return {
_id: testJobId,
Expand Down Expand Up @@ -309,6 +315,14 @@ describe("Job Details Page", () => {
expect(progressBar).toHaveClass("bg-danger");
});
describe("Details", () => {
let urlSpy;
afterEach(() => {
if (urlSpy) {
// making sure the mock is clear even on error,
// otherwise some weird errors start popping up
urlSpy.mockRestore();
}
});
it("Should be collapsed by default", () => {
setupGetJobMock(makeTestJob());
const { container } = render(<JobDetails />);
Expand All @@ -317,6 +331,7 @@ describe("Job Details Page", () => {
});
it("Should open when the toggle button is clicked", () => {
setupGetJobMock(makeTestJob());
setupURLSpyMock(urlSpy);
const { container } = render(<JobDetails />);
const toggleButton = container.querySelector(".job-details-toggle a");
expect(toggleButton).toHaveTextContent("Expand");
Expand All @@ -332,6 +347,7 @@ describe("Job Details Page", () => {
const testJob = makeTestJob();
const serverMetrics = JSON.parse(testJob.server_metrics);
setupGetJobMock(testJob);
setupURLSpyMock(urlSpy);
const { container } = render(<JobDetails />);
const toggleButton = container.querySelector(".job-details-toggle a");
act(() => toggleButton.click());
Expand Down Expand Up @@ -382,6 +398,7 @@ describe("Job Details Page", () => {
const testJob = makeTestJob();
const serverMetrics = JSON.parse(testJob.server_metrics);
setupGetJobMock(testJob);
setupURLSpyMock(urlSpy);
const { container } = render(<JobDetails />);
const progressToggleButton = container.querySelector(".job-details-toggle a");
act(() => progressToggleButton.click());
Expand All @@ -395,6 +412,7 @@ describe("Job Details Page", () => {
const testJob = makeTestJob();
const serverMetrics = JSON.parse(testJob.server_metrics);
setupGetJobMock(testJob);
setupURLSpyMock(urlSpy);
const { container } = render(<JobDetails />);
const progressToggleButton = container.querySelector(".job-details-toggle a");
act(() => progressToggleButton.click());
Expand All @@ -415,6 +433,7 @@ describe("Job Details Page", () => {
const testJob = makeTestJob();
const serverMetrics = JSON.parse(testJob.server_metrics);
setupGetJobMock(testJob);
setupURLSpyMock(urlSpy);
const { container } = render(<JobDetails />);
const progressToggleButton = container.querySelector(".job-details-toggle a");
act(() => progressToggleButton.click());
Expand Down Expand Up @@ -479,6 +498,58 @@ describe("Job Details Page", () => {
);
});
});
describe("Download metrics", () => {
it("Should render the download server metrics button correctly", async () => {
const testJob = makeTestJob();
setupGetJobMock(testJob);
const { container } = render(<JobDetails />);

const testURL = "test url";
urlSpy = setupURLSpyMock(urlSpy, testURL);

const progressToggleButton = container.querySelector(".job-details-toggle a");
act(() => progressToggleButton.click());

const expectedServerMetrics = JSON.stringify(JSON.parse(testJob.server_metrics), null, 4);
expect(urlSpy.createObjectURL).toHaveBeenCalledWith(new Blob([expectedServerMetrics]));

const jobProgressDetailsComponent = container.querySelector(".job-progress-detail");
const downloadMetricsButton = jobProgressDetailsComponent.querySelector(".download-metrics-button");
expect(downloadMetricsButton.getAttribute("href")).toBe(testURL);
expect(downloadMetricsButton.getAttribute("download")).toBe("server-metrics.json");
});
it("Should render the download client metrics button correctly", () => {
const testJob = makeTestJob();
setupGetJobMock(testJob);
const { container } = render(<JobDetails />);

const testURL = "test url";
urlSpy = setupURLSpyMock(urlSpy, testURL);

const testClientIndex = 1;
let toggleButton = container.querySelectorAll(".job-client-progress .job-details-toggle a")[
testClientIndex
];
act(() => toggleButton.click());

const expectedClientMetrics = JSON.stringify(
JSON.parse(testJob.clients_info[testClientIndex].metrics),
null,
4,
);
expect(urlSpy.createObjectURL).toHaveBeenCalledWith(new Blob([expectedClientMetrics]));

const clientProgressDetailsComponent = container.querySelector(
`#job-details-client-config-progress-${testClientIndex} .job-progress-detail`,
);
const downloadMetricsButton =
clientProgressDetailsComponent.querySelector(".download-metrics-button");
expect(downloadMetricsButton.getAttribute("href")).toBe(testURL);
expect(downloadMetricsButton.getAttribute("download")).toBe(
`client-metrics-${testClientIndex}.json`,
);
});
});
describe("Clients", () => {
it("Renders their progress bars correctly", () => {
const testJob = makeTestJob();
Expand All @@ -497,6 +568,7 @@ describe("Job Details Page", () => {
it("Renders the progress details correctly", () => {
const testJob = makeTestJob();
setupGetJobMock(testJob);
setupURLSpyMock(urlSpy);
const { container } = render(<JobDetails />);

let toggleButton = container.querySelectorAll(".job-client-progress .job-details-toggle a")[0];
Expand Down
11 changes: 4 additions & 7 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ uvicorn = {version = "^0.23.2", extras = ["standard"]}
wandb = "^0.18.0"
torchvision = "^0.18.0"
redis = "^5.0.1"
python-multipart = "^0.0.9"
python-multipart = "^0.0.18"
pydantic = "^1.10.15"
motor = "^3.4.0"
tqdm = "^4.66.3"
Expand Down

0 comments on commit 729ba67

Please sign in to comment.