Remove websockets usage in cdap-ui - #1428
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the custom WebSocket-based data source implementation (including MyDataSource, MyCDAPDataSource, mySocket, and the backend Aggregator) and transitions the application to use standard HTTP requests via $http. Feedback on these changes highlights a few critical issues: removing the WebSocket and data source modules has left behind orphaned references to MyDataSourceProvider and MYSOCKET_EVENT in app/hydrator/main.js which will cause runtime errors, and has also left unused imports and helper functions in server.js that should be cleaned up.
| * setting the className based events from $state and caskTheme | ||
| */ | ||
| .controller('BodyCtrl', function ($scope, $cookies, $cookieStore, caskTheme, CASK_THEME_EVENT, $rootScope, $state, $log, MYSOCKET_EVENT, MyCDAPDataSource, MY_CONFIG, MYAUTH_EVENT, EventPipe, myAuth, $window, myAlertOnValium, myLoadingService, myHelpers, $http) { | ||
| .controller('BodyCtrl', function ($scope, $cookies, $cookieStore, caskTheme, CASK_THEME_EVENT, $rootScope, $state, $log, MY_CONFIG, MYAUTH_EVENT, EventPipe, myAuth, $window, myAlertOnValium, myLoadingService, myHelpers, $http) { |
There was a problem hiding this comment.
Removing MYSOCKET_EVENT and MyCDAPDataSource from the controller dependencies is correct, but there are two remaining references in this file that will now cause runtime errors:
MyDataSourceProviderConfiguration (Lines 91-93):
.config(function (MyDataSourceProvider) {
MyDataSourceProvider.defaultInterval = 5;
})Since MyDataSource has been completely removed (from datasource.js), configuring MyDataSourceProvider here will throw an AngularJS injector error on startup. This block should be removed.
MYSOCKET_EVENTEvent Listener (Lines 301-304):
EventPipe.on(MYSOCKET_EVENT.reconnected, function () {
$log.log('[DataSource] reconnected.');
myLoadingService.hideLoadingIcon();
});Since MYSOCKET_EVENT is no longer injected or defined, referencing it here will throw a ReferenceError at runtime. This event listener should be removed.
| }); | ||
| function gracefulShutdown() { | ||
| log.info('Caught SIGTERM. Closing http & ws server'); | ||
| log.info('Caught SIGTERM. Closing http server'); |
There was a problem hiding this comment.
Since the WebSocket upgrade listener has been removed, the helper function getAuthHeaderFromRawCookies (lines 52-57) and the cookie import (line 22) are no longer used anywhere in server.js.
Please remove them to keep the codebase clean and avoid dead code:
import cookie from 'cookie';(line 22)getAuthHeaderFromRawCookiesfunction definition (lines 52-57)
1 participant
Remove websockets usage in cdap-ui
PR Type
Links
Jira: Jira issue #
Test Plan
Screenshots