Skip to content

Refactoring Suggestions

Prasad Talasila edited this page May 13, 2018 · 6 revisions

Coding Standards

  • Use Js closures for function level scoping.
  • module private functions and objects.
  • Jslint in travis pipeline.
  • Use code Auto-formatter for Js.
  • Use nginx for serving static html content.
  • Use Main Server as pure-API.
  • All commits happen through PRs.

Overall Application

  • Create one config file and its parser for the entire AutolabJS project. See issue #268.
  • Reformat travis.yml as per clinical bioportal repository.
  • We need configuration consolidation and validation. See issue #74, issue #250.
  • Detect and use free ports for all the components. See issue #82
  • Retry connections to other components instead of failing and exiting. Some example inter-component connections are: load balancer <--> execution nodes, load balancer <--> main server, load balancer, main server <--> MySQL, load balancer, execution nodes --> gitlab. See issue #65, issue #82
  • HTTPS connection timeouts. See issue #84
  • Impose validation and duplicate detection conditions on lab names. See issue #104, issue #231

Main Server

  • Removal of repeated and unused lines, along with the TESTING flags throughout the main server.
  • Check the validity of all the json files used and handle the errors(if any) suitably.
  • All the functionality of the main server can be split into three parts:-
    1. A database.js to handle sql queries and responses and wrap them in functions.
    2. A labs.js to rename a lab, request a revaluation etc. and to show active status, penalty, scoreboard etc.(read from labs.json)
    3. An admin.js (already present) keeping in mind the limited access a normal user should have.
  • All API calls must be converted from simplex to duplex.
  • Refactor the socket.io service interface of the main server to HTTP REST interface. See issue #300
  • Improve Cookie Generation.
  • Rename flag and other temporary variables to something which is easier to understand. eg. var isValidLab
  • Verify clients using proper https/ssl certificates.
  • Refactoring double declaration of checkConnection request to a single function.
  • We have the admin portal that is used for updating course and lab configurations. However, the main server loads the configuration files at start up and does not read these config files again.
    We need to force the main_server to execute the configuration file reading after configuration update in the admin portal.
  • The lab DB tables need user id number as primary key. See issue #265
  • The config.json of main server has gitlab credentials even though they are not needed. See issue #138.
  • Dynamic reloading of instructor, course and lab configurations. See issue #52.
  • Confirmation message before deleting a lab via admin portal. See issue #62
  • Configurable html links on website. See issue #123

Load Balancer

  • Removal of repeated and unused lines, along with the TESTING flags throughout the load balancer.
  • Modifying the use of core modules sys and exec.
  • Refactoring double declaration of checkConnection request to a single function.
  • Use of ca certificates for two way verification of the https request between the client and the request responder.
  • Job-id inclusion for nodes and job queues.
  • All API calls must be converted from simplex to duplex.
  • Allowing only configured nodes to be added via the addNode request.
  • Keeping SQL connection alive
  • Refactoring start up scripts to support ansible way.
  • All the functionality of the load balancer can be split up into three parts.
    1. Maintenance of the execution node status.
    2. interactions with the MySQL
    3. Communication with the main server and execution nodes
      These can probably placed in three different modules.
  • Each execution node has a scores.json file which has the socket configuration of the execution node. This information is passed to load balancer along with every evaluation result. The node details received in the evaluation result are taken by the load balancer and added to the node_queue array without any checking. This problem needs to be fixed by mapping each socket connection to a execution node. The results received on a socket imply a response from the corresponding execution node.
  • Periodic health checks on all execution nodes and removal of non-functioning execution nodes. See issue #79
  • Allow only authorized execution nodes to be added to the load balancer. See issue #246

Execution Nodes

  • Update execute.sh script to send back phrases as test status. See issue #63.
  • The compile.sh of execution nodes give error in removing test log. See issue #156.
  • all execution nodes use the same SSL certificate for HTTPS API service. See issue #202
  • Convert test_info.txt file into a yaml file. For a discussion on fragility of test_info.txt, see issue #178. The suggested yaml file would contain settings for allowed languages, time limits, quality settings, test types etc.
  • Removal of repeated and unused lines, along with the TESTING flags throughout the execution node.
  • Integrate PM2 module in place of "exec".
  • Proper use of path.join.
  • Replace hard-coded filenames with yaml config files.
  • Use proper JSON object and function to collect request info to avoid long code and make exec-command.(41-43)
  • Dump error,stderr,stdout to log file.
  • Figure out use of array.pop and comment.pop extra spaces.
  • Rename array variable as marks.
  • Minimize temporary variables.
  • Validate config files.
  • Move all variable declarations to the top of the file.
  • Use of lines 132-136.
  • Move server listen to top.
  • Handle all scenarios of response to the request.
  • cleaning up of unnecessary steps in docker-entrypoint.sh of execution node container. see issue #188

Images for socket-io connection:

References

request - Simplified HTTP request client. tutorial
amygdala - RESTful HTTP client for JavaScript powered web applications (has a good way of reusing HTTP options)
crossing - a URL forming library

Clone this wiki locally
You can’t perform that action at this time.