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.

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

Jump to bottom

Actually use configured database #2278

Merged
merged 4 commits into from May 1, 2022
Merged

Actually use configured database #2278

merged 4 commits into from May 1, 2022

Conversation

davidmehren
Copy link
Member

Component/Part

AppModule, logging, db config

Description

This PR

  • Uses the database from the config in AppModule
  • Adapts our config options to follow TypeORM terminology
  • Adds a logging adapter for TypeORM, which logs queries on the DEBUG level

Steps

  • Added implementation
  • Added / updated documentation
  • I read the contribution documentation and
    made sure that:
    • My commits are signed-off to accept the DCO.
    • This PR targets the correct branch: master for 1.x & docs, develop for 2.x

Related Issue(s)

@davidmehren davidmehren added type: feature enhancement An improvement to existing functionality type: feature Adds or requests new functionality labels Apr 18, 2022
@davidmehren davidmehren added this to the Release 2.0 milestone Apr 18, 2022
@davidmehren davidmehren added this to In progress in HedgeDoc 2.0 Backend via automation Apr 18, 2022
@davidmehren davidmehren self-assigned this Apr 18, 2022
@codecov
Copy link

codecov bot commented Apr 18, 2022 •
edited

Codecov Report

Merging #2278 (2aab3a5) into develop (a06a351) will decrease coverage by 0.61%.
The diff coverage is 53.03%.

@@             Coverage Diff             @@
##           develop    #2278      +/-   ##
===========================================
- Coverage    86.03%   85.42%   -0.62%     
===========================================
  Files          143      144       +1     
  Lines         3344     3402      +58     
  Branches       325      343      +18     
===========================================
+ Hits          2877     2906      +29     
- Misses         440      469      +29     
  Partials        27       27
Flag Coverage Δ
e2e-tests 77.51% <53.03%> (-0.48%) ⬇️
integration-tests 76.36% <19.64%> (-1.58%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/logger/typeorm-logger.service.ts 32.55% <32.55%> (ø)
src/logger/console-logger.service.ts 80.59% <83.33%> (+3.01%) ⬆️
src/app.module.ts 100.00% <100.00%> (ø)
src/config/database-type.enum.ts 100.00% <100.00%> (ø)
src/config/database.config.ts 50.00% <100.00%> (ø)
src/config/mock/database.config.mock.ts 100.00% <100.00%> (ø)
src/logger/logger.module.ts 100.00% <100.00%> (ø)
src/utils/session.ts 100.00% <100.00%> (ø)

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 a06a351...2aab3a5. Read the comment docs.

DerMolly
DerMolly requested changes Apr 18, 2022
View reviewed changes
Copy link
Member

@DerMolly DerMolly left a comment

Choose a reason for hiding this comment

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

Maybe we need to remove the sanetize method from the Logger class alltogether and instead use it as an helper anywhere we put user-input (and maybe stuff from the DB) in logs.
Probably a good rule of thumb would be any non-static strings.

src/config/mock/database.config.mock.ts Outdated Show resolved Hide resolved
src/config/database-dialect.enum.ts Show resolved Hide resolved
src/logger/console-logger.service.ts Show resolved Hide resolved
HedgeDoc 2.0 Backend automation moved this from In progress to Review in progress Apr 18, 2022
@davidmehren davidmehren requested a review from DerMolly May 1, 2022 16:25
HedgeDoc 2.0 Backend automation moved this from Review in progress to Reviewer approved May 1, 2022
@davidmehren
refactor(db-config): Use typeorm-style options
0d67e9e 
TypeORM does not use a separate config option for the path
to the SQLite file.
Additionally, the "dialect" is called "type."

This commit adjusts our config to follow the upstream convention
to reduce confusion.

Signed-off-by: David Mehren <git@herrmehren.de>
@davidmehren
feat: Use real database config for app
fe3f464 
Signed-off-by: David Mehren <git@herrmehren.de>
@davidmehren
feat(console-logger): allow to bring own colors
f12ccff 
Signed-off-by: David Mehren <git@herrmehren.de>
@davidmehren
feat: add logging for typeorm
2aab3a5 
Signed-off-by: David Mehren <git@herrmehren.de>
@davidmehren davidmehren merged commit ec398eb into develop May 1, 2022
@davidmehren davidmehren deleted the feat/db_from_config branch May 1, 2022 18:49
HedgeDoc 2.0 Backend automation moved this from Reviewer approved to Done May 1, 2022
davidmehren pushed a commit that referenced this pull request Oct 30, 2022
@tamo @davidmehren
Update HD_DATABASE_*
af0b34e 
Fix leftovers from #2278 cd5256d

Signed-off-by: Tamotsu Takahashi <ttakah+github@gmail.com>
davidmehren pushed a commit that referenced this pull request Nov 20, 2022
@renovate
Update dependency @reduxjs/toolkit to v1.8.4 (#2278)
f1c7bf0 
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@InnayTool InnayTool InnayTool approved these changes

@DerMolly DerMolly DerMolly approved these changes

Assignees

@davidmehren davidmehren

Labels
type: feature enhancement An improvement to existing functionality type: feature Adds or requests new functionality
Projects
No open projects
HedgeDoc 2.0 Backend
Done
Milestone
Version 2.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@davidmehren @InnayTool @DerMolly