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

[WIP] Implements show commands #117

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

mukkachaitanya
Copy link
Collaborator

Requirements

  • Need for new commands for AutolabJS services

Description of the Change

Benefits

Possible Drawbacks

Applicable Issues

@mukkachaitanya
Copy link
Collaborator Author

Output of autolabjs show status command :

Output
| Fetching request results, please wait...
 { components:
   [ { role: 'execution_node', hostname: '10.1.*.***', port: '8091', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8092', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8093', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8094', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8095', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8096', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8097', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8098', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8099', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8100', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8101', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8102', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8103', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8104', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8105', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8106', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8107', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8108', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8109', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8110', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8091', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8092', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8093', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8094', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8095', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8096', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8097', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8098', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8099', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8100', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8101', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8102', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8103', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8104', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8105', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8106', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8107', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8108', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8109', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8110', status: 'down' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8091', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8092', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8093', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8094', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8095', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8096', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8097', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8098', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8099', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8100', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8101', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8102', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8103', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8104', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8105', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8106', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8107', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8108', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8109', status: 'up' },
     { role: 'execution_node', hostname: '10.1.*.***', port: '8110', status: 'up' },
     { role: 'load_balancer',
       hostname: 'autolab.bits-goa.ac.in',
       port: '8081',
       cmd: 'log',
       status: 'up' } ],
  job_queue_length: 0,
  timestamp: 'Wed Jan 23 2019 12:58:55 GMT+0530 (IST)' }
(hostnames hidded purposefully)

Copy link
Member

@prasadtalasila prasadtalasila left a comment

Choose a reason for hiding this comment

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

@mukkachaitanya Please see the suggested changes.

@@ -0,0 +1,16 @@
const getInput = async (args) => {
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need

const inquirer = require('inquirer');
const PromptGenerator = require('../../utils/PromptGenerator');

How did the output come without these requires?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have only implemented the status command, sir. That needs no prompt.

};

const onShow = async (args, options, logger) => {
logger.moduleLog('info', 'Exit', 'Show command invoked.');
Copy link
Member

Choose a reason for hiding this comment

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

This may be logger.moduleLog('info', 'Show', 'Show command invoked.');

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixing it.

const showOptions = await showInput.getInput(args, options);
const validatedOptions = showValidate.validate(showOptions);

logger.moduleLog('debug', 'Eval', 'Show request for');
Copy link
Member

Choose a reason for hiding this comment

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

These two log statements again belong to Show module, not Eval and the statements can be better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixing it.

@@ -0,0 +1,10 @@
const validate = (showEvent) => {
Copy link
Member

Choose a reason for hiding this comment

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

There is no validation logic here. Are you planning to add more later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, sir. The status command had not validation.

spinner.stop();
};

const sendOutput = (event) => {
Copy link
Member

Choose a reason for hiding this comment

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

Starting with this PR, perhaps we can move away from arrow functions to proper functions where there is a bulky function. We had this discussion during the refactoring of tests. The added benefit the the following flow in a module.

module.exports...
//functions arranged to follow top-down rule

@prasadtalasila
Copy link
Member

@mukkachaitanya I can't see the presentation code in lib/model/show.js which actually hides the IP addresses of the execution nodes.

@mukkachaitanya
Copy link
Collaborator Author

@mukkachaitanya I can't see the presentation code in lib/model/show.js which actually hides the IP addresses of the execution nodes.

The IP is hidden here on GitHub. The actual code output doesn't sir.

@codecov
Copy link

codecov bot commented Jan 30, 2019

Codecov Report

Merging #117 into dev will decrease coverage by 0.09%.
The diff coverage is 98.75%.

Impacted file tree graph

@@            Coverage Diff            @@
##              dev     #117     +/-   ##
=========================================
- Coverage   99.28%   99.18%   -0.1%     
=========================================
  Files          24       29      +5     
  Lines         695      856    +161     
=========================================
+ Hits          690      849    +159     
- Misses          5        7      +2
Flag Coverage Δ
#integration 93.92% <91.3%> (-0.32%) ⬇️
#unit 99.18% <98.75%> (-0.1%) ⬇️
Impacted Files Coverage Δ
lib/cli/input/show.js 100% <100%> (ø)
lib/controller/validate/show.js 100% <100%> (ø)
lib/controller/show.js 100% <100%> (ø)
lib/controller/index.js 100% <100%> (ø) ⬆️
lib/cli/output/show.js 100% <100%> (ø)
lib/model/show.js 95.83% <95.83%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0534f8...afde03d. Read the comment docs.

@mukkachaitanya
Copy link
Collaborator Author

Output of autolabjs show score command :
image

(ID hidden purposefully)

const file = '/tmp/autolabjs/scoreboard';
try {
await fs.outputFile(file, scoreboard);
spawn('cat /tmp/autolabjs/scoreboard | less -r', {
Copy link
Collaborator Author

@mukkachaitanya mukkachaitanya Jan 30, 2019

Choose a reason for hiding this comment

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

exec() spawns a shell then executes the command within that shell, buffering any generated output to the callback provides. That is the reason I have used spawn() instead.

@prasadtalasila
Copy link
Member

prasadtalasila commented Jan 30, 2019

@mukkachaitanya The output of autolabjs show score looks good. Please remember to add the display score option for a user (autolabjs show score -l lab_name -i student_id ).

@mukkachaitanya
Copy link
Collaborator Author

@mukkachaitanya The output of autolabjs show score looks good. Please remember to add the display score option for a user (autolabjs show score -l lab_name -i student_id ).

That is also implemented, sir.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants