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
Fix Overview tab is present under Storage section #351
Fix Overview tab is present under Storage section #351
Conversation
/retest |
de3c310
to
9d3491a
Compare
blockPoolsError || | ||
compressionLoadError || | ||
rawCapLoadError; | ||
const loaded = blockPoolsLoaded && (cephLoaded || scLoaded); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix for blockpool page shows not found, I am fixing this part of this because this is also related to hybrid console and this is also a blocker issue
@@ -103,8 +103,8 @@ export const ObjectServiceDetailsCard: React.FC<{}> = () => { | |||
title={t('System name')} | |||
isLoading={!systemResult || !dashboardLinkResult} | |||
error={ | |||
systemLoadError || | |||
dashboardLinkLoadError || | |||
systemLoadError?.message || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here object dashbaord goes blank
scripts/generatePluginPackage.js
Outdated
@@ -0,0 +1,16 @@ | |||
const fs = require('fs'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this ts file. Use ts-node.
@@ -15,14 +15,6 @@ | |||
} | |||
} | |||
}, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets move this file also inside of odf and make console-extension a generated file. Also add it to gitignore as well. Also make it part of install so that it doesn't cause users confusion. We also need to update the readme about adding extensions to packages instead of in the main console-extensions file.
583ba7a
to
6373563
Compare
scripts/generatePluginPackage.ts
Outdated
@@ -0,0 +1,8 @@ | |||
/* Copyright Contributors to the Open Cluster Management project */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* Copyright Contributors to the Open Cluster Management project */ |
|
||
const config: webpack.Configuration & DevServerConfiguration = { | ||
mode: 'development', | ||
entry: {}, | ||
output: { | ||
path: path.resolve(__dirname, 'dist'), | ||
path: path.resolve('./dist'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets compile it to one place? Or is that going to be an issue to run the two plugins concurrently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right now we dont have a concurrent use case, but ya with this solution concurrent is also possible. I am compiling inside corresponding plugin folder and it is considered as a root folder
package.json
Outdated
"build": "NODE_ENV=production PLUGIN=odf I8N_NS=plugin__odf-console yarn build:plugin", | ||
"build-mco": "NODE_ENV=production PLUGIN=mco I8N_NS=plugin__odf-multicluster-console yarn build:plugin", | ||
"build-dev": "NODE_ENV=production PLUGIN=odf I8N_NS=plugin__odf-console yarn build:plugin", | ||
"server:plugin": "cd plugins/${PLUGIN} && ts-node ../../scripts/generatePluginPackage.ts && yarn clean && NODE_ENV={MODE} I8N_NS=${I8N_NS} yarn ts-node ../../node_modules/.bin/webpack serve -c ../../webpack.config.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"server:plugin": "cd plugins/${PLUGIN} && ts-node ../../scripts/generatePluginPackage.ts && yarn clean && NODE_ENV={MODE} I8N_NS=${I8N_NS} yarn ts-node ../../node_modules/.bin/webpack serve -c ../../webpack.config.ts", | |
"server:plugin": "cd plugins/${PLUGIN} && ts-node ../../scripts/generatePluginPackage.ts && yarn clean && NODE_ENV={MODE} I8N_NS=${I8N_NS} yarn ts-node ../../node_modules/.bin/webpack serve", |
Webpack should recursively look for the config file? Is it not able to find it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not finding, becuase not it is invoked from plugins directory, not root directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried, but I am getting errors
Can't resolve './src' in '/home/gshanmug/Workspace/Openshift/odf-console/plugins/mco
Can't resolve './src' in '/home/gshanmug/Workspace/Openshift/odf-console/plugins/odf
plugins/mco/console-plugin.json
Outdated
"displayName": "ODF(OpenShift Data Foundation) Plugin", | ||
"description": "Console plugin for ODF", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"displayName": "ODF(OpenShift Data Foundation) Plugin", | |
"description": "Console plugin for ODF", | |
"displayName": "MCO Plugin", | |
"description": "Console plugin for MCO", |
webpack.config.ts
Outdated
@@ -5,25 +5,20 @@ import { ConsoleRemotePlugin } from '@openshift-console/dynamic-plugin-sdk-webpa | |||
import * as CopyWebpackPlugin from 'copy-webpack-plugin'; | |||
import * as webpack from 'webpack'; | |||
import type { Configuration as DevServerConfiguration } from 'webpack-dev-server'; | |||
import MergeJsonWebpackPlugin from 'merge-jsons-webpack-plugin'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import MergeJsonWebpackPlugin from 'merge-jsons-webpack-plugin'; |
not used anywhere ?
package.json
Outdated
"build": "NODE_ENV=production PLUGIN=odf I8N_NS=plugin__odf-console yarn build:plugin", | ||
"build-mco": "NODE_ENV=production PLUGIN=mco I8N_NS=plugin__odf-multicluster-console yarn build:plugin", | ||
"build-dev": "NODE_ENV=production PLUGIN=odf I8N_NS=plugin__odf-console yarn build:plugin", | ||
"server:plugin": "cd plugins/${PLUGIN} && ts-node ../../scripts/generatePluginPackage.ts && yarn clean && NODE_ENV={MODE} I8N_NS=${I8N_NS} yarn ts-node ../../node_modules/.bin/webpack serve -c ../../webpack.config.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NODE_ENV={MODE}
this "MODE" is it some default env variable ? because we are not exporting it ourselves anywhere...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also do we need to NODE_ENV={MODE} I8N_NS=${I8N_NS}
again during serving ? we are already exporting these during build command...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build command is for production and service is for dev setup which we are running locally, so yes we need this on both the places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay... but "MODE" we have not initialized this variable anywhere...
package.json
Outdated
"build": "NODE_ENV=production PLUGIN=odf I8N_NS=plugin__odf-console yarn build:plugin", | ||
"build-mco": "NODE_ENV=production PLUGIN=mco I8N_NS=plugin__odf-multicluster-console yarn build:plugin", | ||
"build-dev": "NODE_ENV=production PLUGIN=odf I8N_NS=plugin__odf-console yarn build:plugin", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"yarn build" and "yarn build-dev" are exactly same...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build-dev should not have NODE_ENV=production, i will change it
6e267ee
to
ad1c1bc
Compare
Signed-off-by: Gowtham Shanmugasundaram <gshanmug@redhat.com>
ad1c1bc
to
8919f51
Compare
/approve |
/test odf-console-e2e-aws |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bipuladh, GowthamShanmugam The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test images |
/test odf-console-e2e-aws |
/cherrypick release-4.11 |
@bipuladh: new pull request created: #362 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/cherrypick release-4.11-compatibility |
@bipuladh: new pull request created: #363 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2087078
Signed-off-by: Gowtham Shanmugasundaram gshanmug@redhat.com