Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redux integration and error handling #71

Merged
merged 7 commits into from
May 9, 2023
Merged

Conversation

johncalesp
Copy link
Contributor

  • Modified Habitat and Energy messages to handle error messages from deepview profile
  • Added new component for recommendations (commented for now until its complete)
  • Added redux for state management (for now only number of iterations is in the store. Will add more in next PR)
  • Modified unit testing
  • Added limit to throughput silder (now its 65 % of GPU memory)

import { useSelector, useDispatch } from "react-redux";
import { setIterationsValues } from "../redux/slices/trainingScheduleSlice";

const Iterations = () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Iterations has 110 lines of code (exceeds 25 allowed). Consider refactoring.

(item) => item[0] === "demo" && item[1] === 1
);
if (habitatData.error) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@@ -131,6 +144,22 @@ const EnergyConsumption = ({ energyData, numIterations }) => {
};
});
}
if (energyData.error) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

["demo",1]
]
const noHabitatData = {
predictions: [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identical blocks of code found in 2 locations. Consider refactoring.

["A4000", 14.537657],
["RTX4000", 20.04405],
],
habitat: {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@johncalesp johncalesp requested review from jimgao1 and michaelshin May 8, 2023 16:41
event.data.habitat.length === 0 ||
(event.data.habitat[0][0] === "unavailable" &&
event.data.habitat[0][1] === -1.0)
event.data.habitat.predictions &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: in a future PR, we can use redux to get the states

@michaelshin
Copy link
Contributor

I would put the recommendations view in a separate PR and rename this PR to focus on adding redux and handling the error messages

@johncalesp johncalesp changed the title Recommendations view Redux integration and error handling May 9, 2023
const { container } = render(<ProviderPanel numIterations={numIterations} habitatData={noHabitatData} additionalProviders={""}/>);
json: jest.fn().mockResolvedValue(correctData),
});
const { container } = render(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@@ -68,7 +71,11 @@ jest.mock("recharts", () => {

test("Shows loading spinner when there is no habitat data", () => {
// ARRANGE
render(<DeploymentTab numIterations={numIterations} habitatData={[]} />);
render(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@@ -103,7 +109,11 @@ jest.mock("recharts", () => {

test("No energy data: shows only loading spinner", () => {
// ARRANGE
render(<EnergyConsumption energyData={{}} numIterations={numIterations} />);
render(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@codeclimate
Copy link

codeclimate bot commented May 9, 2023

Code Climate has analyzed commit d9f452c and detected 20 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2
Duplication 18

View more on Code Climate.

@johncalesp johncalesp requested a review from michaelshin May 9, 2023 15:29
@johncalesp johncalesp requested a review from michaelshin May 9, 2023 15:33
@johncalesp johncalesp merged commit 45a72b5 into main May 9, 2023
@johncalesp johncalesp deleted the recommendations-view branch May 9, 2023 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants