Skip to content

Commit

Permalink
Code review feedback changes BE and FE
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeyaprakash-NK committed Sep 6, 2024
1 parent 2eb3d98 commit cb818d0
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 58 deletions.
10 changes: 5 additions & 5 deletions dataproc_jupyter_plugin/controllers/dataproc.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ async def get(self):
cluster_list = await client.list_clusters(page_size, page_token)
self.finish(json.dumps(cluster_list))
except Exception as e:
self.log.exception(f"Error fetching cluster list")
self.log.exception(f"Error fetching cluster list: {str(e)}")
self.finish({"error": str(e)})


Expand All @@ -62,7 +62,7 @@ async def get(self):
get_cluster = await client.get_cluster_detail(cluster)
self.finish(json.dumps(get_cluster))
except Exception as e:
self.log.exception(f"Error fetching get cluster")
self.log.exception(f"Error fetching a cluster: {str(e)}")
self.finish({"error": str(e)})


Expand All @@ -75,7 +75,7 @@ async def post(self):
stop_cluster = await client.stop_cluster(cluster)
self.finish(json.dumps(stop_cluster))
except Exception as e:
self.log.exception(f"Error fetching stop cluster")
self.log.exception(f"Error stopping a cluster: {str(e)}")
self.finish({"error": str(e)})


Expand All @@ -88,7 +88,7 @@ async def post(self):
start_cluster = await client.start_cluster(cluster)
self.finish(json.dumps(start_cluster))
except Exception as e:
self.log.exception(f"Error fetching start cluster")
self.log.exception(f"Error starting a cluster: {str(e)}")
self.finish({"error": str(e)})


Expand All @@ -101,5 +101,5 @@ async def delete(self):
delete_cluster = await client.delete_cluster(cluster)
self.finish(json.dumps(delete_cluster))
except Exception as e:
self.log.exception(f"Error deleting cluster")
self.log.exception(f"Error deleting a cluster: {str(e)}")
self.finish({"error": str(e)})
32 changes: 15 additions & 17 deletions dataproc_jupyter_plugin/services/dataproc.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,17 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import proto
import json
import google.oauth2.credentials as oauth2
from google.cloud import dataproc_v1 as dataproc
from google.protobuf.empty_pb2 import Empty
from dataproc_jupyter_plugin import urls
from dataproc_jupyter_plugin.commons.constants import (
CONTENT_TYPE,
DATAPROC_SERVICE_NAME,
)

from google.cloud import dataproc_v1 as dataproc
import proto
import json
import google.oauth2.credentials as oauth2
from google.protobuf.empty_pb2 import Empty


class Client:
def __init__(self, credentials, log, client_session=None):
Expand Down Expand Up @@ -88,11 +87,10 @@ async def list_clusters(self, page_size, page_token):

# Handle the response
async for response in page_result:
clusters_list.append(json.loads(proto.Message.to_json(response)))

clusters_list.append(proto.Message.to_dict(response, use_integers_for_enums=False, preserving_proto_field_name=False))
return clusters_list
except Exception as e:
self.log.exception(f"Error fetching cluster list")
self.log.exception(f"Error fetching cluster list: {str(e)}")
return {"error": str(e)}

async def get_cluster_detail(self, cluster):
Expand All @@ -116,9 +114,9 @@ async def get_cluster_detail(self, cluster):
response = await client.get_cluster(request=request)

# Handle the response
return json.loads(proto.Message.to_json(response))
return proto.Message.to_dict(response, use_integers_for_enums=False, preserving_proto_field_name=False)
except Exception as e:
self.log.exception(f"Error fetching cluster detail")
self.log.exception(f"Error fetching cluster detail: {str(e)}")
return {"error": str(e)}

async def stop_cluster(self, cluster):
Expand All @@ -142,9 +140,9 @@ async def stop_cluster(self, cluster):

response = await operation.result()
# Handle the response
return json.loads(proto.Message.to_json(response))
return proto.Message.to_dict(response, use_integers_for_enums=False, preserving_proto_field_name=False)
except Exception as e:
self.log.exception(f"Error fetching stop cluster")
self.log.exception(f"Error stopping a cluster: {str(e)}")
return {"error": str(e)}

async def start_cluster(self, cluster):
Expand All @@ -168,9 +166,9 @@ async def start_cluster(self, cluster):

response = await operation.result()
# Handle the response
return json.loads(proto.Message.to_json(response))
return proto.Message.to_dict(response, use_integers_for_enums=False, preserving_proto_field_name=False)
except Exception as e:
self.log.exception(f"Error fetching start cluster")
self.log.exception(f"Error starting a cluster: {str(e)}")
return {"error": str(e)}

async def delete_cluster(self, cluster):
Expand All @@ -197,7 +195,7 @@ async def delete_cluster(self, cluster):
if isinstance(response, Empty):
return "Deleted successfully"
else:
return json.loads(proto.Message.to_json(response))
return proto.Message.to_dict(response, use_integers_for_enums=False, preserving_proto_field_name=False)
except Exception as e:
self.log.exception(f"Error deleting cluster")
self.log.exception(f"Error deleting a cluster: {str(e)}")
return {"error": str(e)}
6 changes: 0 additions & 6 deletions src/cluster/clusterServices.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { toast } from 'react-toastify';
import 'react-toastify/dist/ReactToastify.css';
import {
ClusterStatus,
ClusterStatusState,
POLLING_TIME_LIMIT
} from '../utils/const';
import {
Expand Down Expand Up @@ -161,8 +160,6 @@ export class ClusterService {
const serviceURL = `clusterDetail?cluster=${clusterSelected}`;

let responseResult: any = await requestAPI(serviceURL);
responseResult.status.state =
ClusterStatusState[responseResult.status.state.toString()];
if (responseResult) {
if (responseResult.error && responseResult.error.code === 404) {
setErrorView(true);
Expand Down Expand Up @@ -197,9 +194,6 @@ export class ClusterService {
const serviceURL = `clusterDetail?cluster=${selectedCluster}`;

let formattedResponse: any = await requestAPI(serviceURL);
formattedResponse.status.state =
ClusterStatusState[formattedResponse.status.state.toString()];

if (formattedResponse.status.state === ClusterStatus.STATUS_STOPPED) {
ClusterService.startClusterApi(selectedCluster);
clearInterval(timer.current);
Expand Down
25 changes: 12 additions & 13 deletions src/cluster/listCluster.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import clusterErrorIcon from '../../style/icons/cluster_error_icon.svg';
import {
CREATE_CLUSTER_URL,
ClusterStatus,
ClusterStatusState,
STATUS_CREATING,
STATUS_DELETING,
STATUS_ERROR,
Expand Down Expand Up @@ -215,23 +214,23 @@ function ListCluster({
<div
role="button"
aria-disabled={
ClusterStatusState[data.status.state] !== ClusterStatus.STATUS_STOPPED &&
data.status.state !== ClusterStatus.STATUS_STOPPED &&
restartEnabled !== true
}
className={
ClusterStatusState[data.status.state] === ClusterStatus.STATUS_STOPPED &&
data.status.state === ClusterStatus.STATUS_STOPPED &&
restartEnabled !== true
? 'icon-buttons-style'
: 'icon-buttons-style-disable'
}
title="Start Cluster"
onClick={
ClusterStatusState[data.status.state] === ClusterStatus.STATUS_STOPPED && !restartEnabled
data.status.state === ClusterStatus.STATUS_STOPPED && !restartEnabled
? () => ClusterService.startClusterApi(data.clusterName)
: undefined
}
>
{ClusterStatusState[data.status.state] === ClusterStatus.STATUS_STOPPED &&
{data.status.state === ClusterStatus.STATUS_STOPPED &&
!restartEnabled ? (
<iconStart.react
tag="div"
Expand All @@ -254,20 +253,20 @@ function ListCluster({
return (
<div
role="button"
aria-disabled={ClusterStatusState[data.status.state] !== ClusterStatus.STATUS_RUNNING}
aria-disabled={data.status.state !== ClusterStatus.STATUS_RUNNING}
className={
ClusterStatusState[data.status.state] === ClusterStatus.STATUS_RUNNING
data.status.state === ClusterStatus.STATUS_RUNNING
? 'icon-buttons-style'
: 'icon-buttons-style-disable'
}
title="Stop Cluster"
onClick={
ClusterStatusState[data.status.state] === ClusterStatus.STATUS_RUNNING
data.status.state === ClusterStatus.STATUS_RUNNING
? () => ClusterService.stopClusterApi(data.clusterName)
: undefined
}
>
{ClusterStatusState[data.status.state] === ClusterStatus.STATUS_RUNNING ? (
{data.status.state === ClusterStatus.STATUS_RUNNING ? (
<iconStop.react
tag="div"
className="icon-white logo-alignment-style"
Expand All @@ -289,20 +288,20 @@ function ListCluster({
return (
<div
role="button"
aria-disabled={ClusterStatusState[data.status.state] !== ClusterStatus.STATUS_RUNNING}
aria-disabled={data.status.state !== ClusterStatus.STATUS_RUNNING}
className={
ClusterStatusState[data.status.state] === ClusterStatus.STATUS_RUNNING
data.status.state === ClusterStatus.STATUS_RUNNING
? 'icon-buttons-style'
: 'icon-buttons-style-disable'
}
title="Restart Cluster"
onClick={
ClusterStatusState[data.status.state] === ClusterStatus.STATUS_RUNNING
data.status.state === ClusterStatus.STATUS_RUNNING
? () => restartClusterApi(data.clusterName)
: undefined
}
>
{ClusterStatusState[data.status.state] === ClusterStatus.STATUS_RUNNING ? (
{data.status.state === ClusterStatus.STATUS_RUNNING ? (
<iconRestart.react
tag="div"
className="icon-white logo-alignment-style"
Expand Down
14 changes: 1 addition & 13 deletions src/utils/const.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,16 +256,4 @@ export const TIER_SELECT_OPTIONS = [
{ key: 'standard', value: 'standard', text: 'standard' },
{ key: 'premium', value: 'premium', text: 'premium' }
];
export const ClusterStatusState: any = {
0: 'UNKNOWN',
1: STATUS_CREATING,
2: STATUS_RUNNING,
3: STATUS_ERROR,
4: STATUS_DELETING,
5: 'UPDATING',
6: STATUS_STOPPING,
7: STATUS_STOPPED,
8: STATUS_STARTING,
9: 'ERROR_DUE_TO_UPDATE',
10: 'REPAIRING'
};

8 changes: 4 additions & 4 deletions src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ import { requestAPI } from '../handler/handler';
import {
API_HEADER_BEARER,
API_HEADER_CONTENT_TYPE,
ClusterStatusState,
DCU_HOURS,
GB_MONTHS,
HTTP_METHOD,
PYSPARK,
SPARK,
SPARKR,
SPARKSQL,
STATUS_CREATING,
STATUS_DONE,
STATUS_ERROR,
STATUS_FAIL,
Expand Down Expand Up @@ -262,11 +262,11 @@ export const statusMessage = (data: { status: { state: string } }) => {
}
};

export const statusValue = (data: { status: { state: number } }) => {
if (data.status.state === 1) {
export const statusValue = (data: { status: { state: string } }) => {
if (data.status.state === STATUS_CREATING) {
return STATUS_PROVISIONING;
} else {
return ClusterStatusState[data.status.state];
return data.status.state;
}
};

Expand Down

0 comments on commit cb818d0

Please sign in to comment.