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

Refactor existing database code into a genuine DAL module #950

Closed
dondi opened this issue Jan 11, 2022 · 8 comments
Closed

Refactor existing database code into a genuine DAL module #950

dondi opened this issue Jan 11, 2022 · 8 comments

Comments

@dondi
Copy link
Owner

dondi commented Jan 11, 2022

Our existing database code needs a refactor to a full-fledged DAL, especially if our database capabilities will continue to expand.

@Onariaginosa
Copy link
Collaborator

I think I created a sort of DAL. I isolated the querying of the expression database and getting a response into a file called grnsight-dal.js and made the database-controller compatible with querying to the network database as well. I also cleaned up the code a bit so there is less repetitions. This is in the Onariaginosa branch.

@dondi
Copy link
Owner Author

dondi commented Feb 15, 2022

Upon review of the work so far, the direction looks good and next steps are:

  1. Pull out the database code from the database-controller.js file and place it in dal/expression-database.js (or similar), with database-controller.js being renamed and very likely getting a lot smaller than it is now
  2. The work done in grnsight-dal.js actually pertains to the API rather than the database and has been separated out as a new issue Consolidate existing web app API calls into an abstraction layer #957

@Onariaginosa
Copy link
Collaborator

This is complete

@dondi
Copy link
Owner Author

dondi commented Mar 8, 2022

Code is reviewed and will go out with the next GRNsight release.

@dondi dondi closed this as completed Mar 8, 2022
@dondi
Copy link
Owner Author

dondi commented Mar 8, 2022

DAL code was merged into beta but when it was deployed to the GRNsight server it produced an error relating to a new import statement—we’ll need to look into this.

@dondi dondi reopened this Mar 8, 2022
@dondi
Copy link
Owner Author

dondi commented Mar 8, 2022

In order to try a new deployment, make sure to do the following:

  • git checkout beta
  • git pull

This is because we reverted beta to a prior state in order to keep the service running.

@dondi
Copy link
Owner Author

dondi commented Sep 7, 2022

Need to review status of this with @Onariaginosa

@dondi dondi self-assigned this Sep 7, 2022
@dondi
Copy link
Owner Author

dondi commented Sep 14, 2022

Confirmed as finished

@dondi dondi closed this as completed Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants