Skip to content

removed ODO log command #4566

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.

Sign up for GitHub

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

Merged
msivasubramaniaan merged 3 commits into from
Oct 18, 2024
Merged

removed ODO log command #4566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
496d32d
removed odo log command
msivasubramaniaan Oct 17, 2024
135315f
removed odo log command from test files
msivasubramaniaan Oct 17, 2024
e704971
removed odo log command
msivasubramaniaan Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 1 addition & 51 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,6 @@
"onCommand:openshift.component.describe",
"onCommand:openshift.component.openInBrowser",
"onCommand:openshift.component.describe.palette",
"onCommand:openshift.component.log",
Copy link
Copy Markdown
Collaborator Author

@msivasubramaniaan msivasubramaniaan Oct 17, 2024

Choose a reason for hiding this comment

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

@vrubezhny I removed the command as well

"onCommand:openshift.component.log.palette",
"onCommand:openshift.component.followLog",
"onCommand:openshift.component.followLog.palette",
"onCommand:openshift.componentTypesView.registry.closeView",
"onCommand:openshift.component.debug",
"onCommand:openshift.component.debug.palette",
Expand Down Expand Up @@ -563,26 +559,6 @@
"title": "Describe Component",
"category": "OpenShift"
},
{
"command": "openshift.component.log",
"title": "Show Log",
"category": "OpenShift"
},
{
"command": "openshift.component.log.palette",
"title": "Show Component Log",
"category": "OpenShift"
},
{
"command": "openshift.component.followLog",
"title": "Follow Log",
"category": "OpenShift"
},
{
"command": "openshift.component.followLog.palette",
"title": "Follow Component Log",
"category": "OpenShift"
},
{
"command": "openshift.component.openInBrowser",
"title": "Open in Browser",
Expand Down Expand Up @@ -1181,14 +1157,6 @@
"command": "openshift.component.describe",
"when": "view == openshiftComponentsView"
},
{
"command": "openshift.component.log",
"when": "view == openshiftProjectExplorer"
},
{
"command": "openshift.component.followLog",
"when": "view == openshiftProjectExplorer"
},
{
"command": "openshift.component.commands.command.run",
"when": "view == openshiftComponentsView"
Expand Down Expand Up @@ -1285,14 +1253,6 @@
"command": "openshift.component.describe.palette",
"when": "false"
},
{
"command": "openshift.component.followLog.palette",
"when": "false"
},
{
"command": "openshift.component.log.palette",
"when": "false"
},
{
"command": "openshift.component.revealContextInExplorer",
"when": "false"
Expand Down Expand Up @@ -1813,16 +1773,6 @@
"when": "view == openshiftComponentsView && viewItem =~ /openshift\\.component.*/",
"group": "c3@1"
},
{
"command": "openshift.component.log",
"when": "view == openshiftComponentsView && viewItem =~ /openshift\\.component.*\\.dev-run.*/",
"group": "c3@2"
},
{
"command": "openshift.component.followLog",
"when": "view == openshiftComponentsView && viewItem =~ /openshift\\.component.*\\.dev-run.*/",
"group": "c3@3"
},
{
"command": "openshift.component.deleteConfigurationFiles",
"when": "view == openshiftComponentsView && viewItem =~ /openshift\\.component.*\\.dev-nrn.*/",
Expand Down Expand Up @@ -1959,7 +1909,7 @@
},
{
"command": "openshift.resource.watchLogs",
"when": "view == openshiftProjectExplorer && viewItem =~ /^openshift\\.k8sObject\\.(?!helm|pod)/"
"when": "view == openshiftProjectExplorer && viewItem =~ /^openshift\\.k8sObject\\.(?!helm)/"
},
{
"command": "openshift.deployment.shell",
Expand Down
2 changes: 1 addition & 1 deletion src/explorer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ export class OpenShiftExplorer implements TreeDataProvider<ExplorerItem>, Dispos
let intervalId: ReturnType<typeof setInterval> | undefined = undefined;

function checkForPod() {
void Oc.Instance.getLogs('Deployment', component.metadata.name, namespace).then((logs) => {
void Oc.Instance.getLogs(`${component.kind}`, component.metadata.name, namespace).then((logs) => {
clearInterval(intervalId);
resolve();
}).catch(_e => { });
Expand Down
14 changes: 0 additions & 14 deletions src/odo/command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,6 @@ export class Command {
return Command.describeComponent().addOption(new CommandOption('-o', 'json', false));
}

static showLog(platform?: string): CommandText {
const result = new CommandText('odo', 'logs', [
new CommandOption('--dev'),
]);
if (platform) {
result.addOption(new CommandOption('--platform', platform));
}
return result;
}

static showLogAndFollow(platform?: string): CommandText {
return Command.showLog(platform).addOption(new CommandOption('--follow'));
}

@verbose
static createLocalComponent(
devfileType = '', // will use empty string in case of undefined devfileType passed in
Expand Down
20 changes: 0 additions & 20 deletions src/openshift/component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,26 +430,6 @@ export class Component extends OpenShiftItem {
`Describe '${componentFolder.component.devfileData.devfile.metadata.name}' Component`);
}

@vsCommand('openshift.component.log', true)
static async log(componentFolder: ComponentWorkspaceFolder): Promise<void> {
const componentName = componentFolder.component.devfileData.devfile.metadata.name;
const showLogCmd = Command.showLog(Component.getDevPlatform(componentFolder));
Copy link
Copy Markdown
Contributor

@vrubezhny vrubezhny Oct 17, 2024

Choose a reason for hiding this comment

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

Shouldn't we remove the command itself then?

await OpenShiftTerminalManager.getInstance().executeInTerminal(
showLogCmd,
componentFolder.contextPath,
`Show '${componentName}' Component Log`);
}

@vsCommand('openshift.component.followLog', true)
static async followLog(componentFolder: ComponentWorkspaceFolder): Promise<void> {
const componentName = componentFolder.component.devfileData.devfile.metadata.name;
const showLogCmd = Command.showLogAndFollow(Component.getDevPlatform(componentFolder));
Copy link
Copy Markdown
Contributor

@vrubezhny vrubezhny Oct 17, 2024

Choose a reason for hiding this comment

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

Same here. Shouldn't we remove the command itself?

await OpenShiftTerminalManager.getInstance().executeInTerminal(
showLogCmd,
componentFolder.contextPath,
`Follow '${componentName}' Component Log`);
}

@vsCommand('openshift.component.openCreateComponent')
static async createComponent(): Promise<void> {
await CreateComponentLoader.loadView('Create Component');
Expand Down
42 changes: 0 additions & 42 deletions test/integration/command.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,48 +165,6 @@ suite('odo commands integration', function () {
term.dispose();
});

test('showLog()', async function () {
await ODO.execute(Command.showLog(), componentLocation);
});

test('showLogAndFollow()', async function() {
const outputEmitter = new EventEmitter<string>();
let devProcess: ChildProcess;
function failListener(_error) {
assert.fail('showLogAndFollow() errored before it was closed');
}
const term = window.createTerminal({
name: 'test terminal',
pty: {
open: () => {
void CliChannel.getInstance().spawnTool(Command.showLogAndFollow()) //
.then(childProcess => {
devProcess = childProcess
devProcess.on('error', failListener);
});
},
close: () => {
if (devProcess) {
devProcess.removeListener('error', failListener);
devProcess.kill('SIGINT');
}
},
handleInput: (data: string) => {
if (data.length) {
if (devProcess) {
devProcess.removeListener('error', failListener);
devProcess.kill('SIGINT');
}
}
},
onDidWrite: outputEmitter.event
}
});
await new Promise<void>(resolve => setTimeout(resolve, 1000));
// we instruct the pseudo terminal to close the dev session when any text is sent
term.sendText('a');
term.dispose();
});
});

suite('component dev', function() {
Expand Down
30 changes: 0 additions & 30 deletions test/ui/suite/componentContextMenu.ts
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@lgrossma lgrossma Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is it necessary to delete these two tests entirely? If odo log gets replaced by different solution, these tests could be still used.. (Maybe some changes would be needed, but the workflow could stay the same)

Copy link
Copy Markdown
Collaborator Author

@msivasubramaniaan msivasubramaniaan Oct 18, 2024

Choose a reason for hiding this comment

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

As plan to get rid of ODO, I removed the whole code and these log functionality is working through oc logs. But not in the component level. It will be on deployment and pod level.

Copy link
Copy Markdown
Contributor

@lgrossma lgrossma Oct 18, 2024

Choose a reason for hiding this comment

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

I still think it is pretty handy to be able to check the logs in component view and not having to look for the right deployment in the deployments view..

Original file line number Diff line number Diff line change
Expand Up @@ -191,36 +191,6 @@ export function testComponentContextMenu() {
expect(tabName).to.contain(expectedTabName);
});

it('Show log works', async () => {
//open menu and select show log
const contextMenu = await component.openContextMenu();
await contextMenu.select(MENUS.showLog);

//check for active tab name
const tabName = await openshiftTerminal.getActiveTabName();
expect(tabName).to.contain(`Show '${componentName}' Component Log`);

//check for terminal content
const terminalText = await openshiftTerminal.getTerminalText();
expect(terminalText).to.contain('runtime: App started on PORT');
expect(terminalText).to.contain('Press any key to close this terminal');
});

it('Follow log works', async () => {
//open menu and select follow log
const contextMenu = await component.openContextMenu();
await contextMenu.select(MENUS.followLog);

//check for active tab name
const tabName = await openshiftTerminal.getActiveTabName();
expect(tabName).to.contain(`Follow '${componentName}' Component Log`);

//check for terminal text
const terminalText = await openshiftTerminal.getTerminalText();
expect(terminalText).to.contain('runtime: App started on PORT');
expect(terminalText).not.to.contain('Press any key to close this terminal');
});

it('Debug works', async () => {
this.timeout(80_000);

Expand Down
34 changes: 0 additions & 34 deletions test/unit/openshift/component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const { expect } = chai;
chai.use(sinonChai);

suite('OpenShift/Component', function () {
let quickPickStub: sinon.SinonStub;
let sandbox: sinon.SinonSandbox;
let termStub: sinon.SinonStub; let execStub: sinon.SinonStub;
const fixtureFolder = path.join(__dirname, '..', '..', '..', 'test', 'fixtures').normalize();
Expand Down Expand Up @@ -302,39 +301,6 @@ suite('OpenShift/Component', function () {
});
});

suite.skip('log', function () {

test('log calls the correct odo command', async function () {
await Component.log(componentItem1);
expect(termStub).calledOnceWith(Command.showLog());
});

test('works with no context', async () => {
await Component.log(null);
expect(termStub).calledOnceWith(Command.showLog());
});
});

suite.skip('followLog', function() {

test('returns null when cancelled', async function () {
quickPickStub.onFirstCall().resolves();
const result = await Component.followLog(null);

expect(result).null;
});

test('followLog calls the correct odo command', async function () {
await Component.followLog(componentItem1);
expect(termStub).calledOnceWith(Command.showLogAndFollow());
});

test('works with no context', async function () {
await Component.followLog(null);
expect(termStub).calledOnceWith(Command.showLogAndFollow());
});
});

suite.skip('debug', () => {
test('without context exits if no component selected', async () => {
const result = await Component.debug(undefined);
Expand Down
⚡