url cleanup upgrade #148

shay123g · 2021-03-13T16:29:24Z

Closes #16

codecov · 2021-03-13T16:29:27Z

Codecov Report

Merging #148 (75c9a8f) into release/0.2.0 (5afc392) will increase coverage by 0.51%.
The diff coverage is 25.92%.

@@                Coverage Diff                @@
##           release/0.2.0     #148      +/-   ##
=================================================
+ Coverage          18.11%   18.63%   +0.51%     
=================================================
  Files                 36       36              
  Lines               1325     1358      +33     
  Branches              46       47       +1     
=================================================
+ Hits                 240      253      +13     
- Misses              1057     1077      +20     
  Partials              28       28

Impacted Files	Coverage Δ
src/config/index.ts	`0.00% <0.00%> (ø)`
src/config/types.ts	`0.00% <0.00%> (ø)`
src/index.ts	`0.00% <0.00%> (ø)`
src/services/storage/drivers/inMemory/index.ts	`0.00% <0.00%> (ø)`
src/services/storage/drivers/relational/index.ts	`0.00% <0.00%> (ø)`
src/services/storage/drivers/relational/types.ts	`0.00% <0.00%> (ø)`
src/services/storage/index.ts	`0.00% <0.00%> (ø)`
src/config/validate.ts	`91.52% <75.00%> (-1.21%)`	⬇️
src/config/__test__/helpers.ts	`100.00% <100.00%> (ø)`
src/config/normalize.ts	`100.00% <100.00%> (ø)`
... 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 5afc392...75c9a8f. Read the comment docs.

SnirShechter · 2021-03-13T17:45:03Z

@shay123g

That's only for the InMemory driver. What about the Relational driver?

Also, we've talked about a possible flag in the config to specify whether to delete from created_at date or from last_used date.

shay123g · 2021-03-13T18:16:50Z

@shay123g

That's only for the InMemory driver. What about the Relational driver?

Also, we've talked about a possible flag in the config to specify whether to delete from created_at date or from last_used date.

I will fix
Relational - @Danielg212 still working on it, he need to complete the fix for deleteOverdue

into release/0.2.0 � Conflicts: � src/services/storage/drivers/inMemory/index.ts

shay123g · 2021-03-14T12:18:08Z

@SnirShechter pls check

.env.development

src/services/storage/drivers/inMemory/index.ts

Co-authored-by: lesagi <lesagi@gmail.com>

lesagi

nothing crucial, everything looks fine
the .env I changed to the same format of choosing one option out of many as the 'storage' has

in the relativeDate, I'm trying to use const as much as I can, but that's just a general preference, you can ignore it if you think it's less readable

shay123g · 2021-03-16T14:14:23Z

@lesagi I already accept your changes and commited them.. thanks for the explanation though. I asked for review again to check those changes and approve\request more changes

SnirShechter · 2021-03-13T17:45:45Z

src/services/storage/drivers/inMemory/index.ts

 		}
 		public async deleteOverdue(timespanMs: number): Promise<number> {
 			const deleteBefore = new Date().getTime() - timespanMs
 			let deletedCount = 0


See the comment in the PR

src/config/validate.ts

SnirShechter · 2021-03-19T09:25:37Z

src/services/storage/drivers/inMemory/index.ts

-import type { StoredUrl, UrlWithInformation, UrlRequestData, UrlInformation } from '../../types/url.js'
+import type {StoredUrl, UrlWithInformation, UrlRequestData, UrlInformation} from '../../types/url.js'
+import {logger} from "../../../logger/logger";
+import {getRawConfig} from "../../../../config/__test__/helpers";


It's better to pass the expireFrom via the driver configurations instead of importing the config.

Also, you imported functions from the tests.

SnirShechter · 2021-03-19T09:26:02Z

src/services/storage/drivers/inMemory/index.ts

+				if (relativeDate <= deleteBefore) {
 					this.storage.data.urls.delete(storedUrl.id)
 					deletedCount++
 				}


Where's the Relational implementation?

SnirShechter · 2021-03-19T09:27:29Z

src/config/validate.ts


+function validateUrlExpireFrom(urlExpire: string): void {
+	logger.debug(`Start validateUrlExpireFrom with ${urlExpire}`)
+	if (!urlExpire || (urlExpire != 'create' && urlExpire != 'update')) {


The feature we talked about is "expire from either 'create' or 'last used' ". You should add the 'last used'.

Co-authored-by: Snir Shechter <me@snir.sh>

into 16 � Conflicts: � src/config/types.ts � src/index.ts � src/services/storage/drivers/inMemory/index.ts � src/services/storage/drivers/relational/index.ts

shay123g · 2021-03-29T11:56:00Z

@SnirShechter conflicts resolved

#16

756fc02

shay123g requested review from Danielg212, SnirShechter and lesagi March 13, 2021 16:29

Shay.Gazit added 3 commits March 13, 2021 21:01

CR fixes

3720c2e

Merge branch 'release/0.2.0' of https://github.com/mini-services/miniurl

f05a3aa

into release/0.2.0 � Conflicts: � src/services/storage/drivers/inMemory/index.ts

resolve conflict

e3d2c32

shay123g requested review from SnirShechter and lesagi and removed request for Danielg212, SnirShechter and lesagi March 16, 2021 11:13

lesagi reviewed Mar 16, 2021

View reviewed changes

.env.development Outdated Show resolved Hide resolved

lesagi reviewed Mar 16, 2021

View reviewed changes

src/services/storage/drivers/inMemory/index.ts Outdated Show resolved Hide resolved

shay123g requested a review from lesagi March 16, 2021 11:38

shay123g and others added 2 commits March 16, 2021 13:39

Update .env.development

9369ee5

Co-authored-by: lesagi <lesagi@gmail.com>

Update src/services/storage/drivers/inMemory/index.ts

6ab1042

Co-authored-by: lesagi <lesagi@gmail.com>

lesagi reviewed Mar 16, 2021

View reviewed changes

shay123g requested a review from lesagi March 16, 2021 14:14

fix test & config

71e967f

shay123g changed the title ~~#16~~ url cleanup upgrade Mar 17, 2021

shay123g linked an issue Mar 17, 2021 that may be closed by this pull request

Url cleanup upgrades #16

Closed

SnirShechter requested changes Mar 19, 2021

View reviewed changes

shay123g and others added 2 commits March 22, 2021 10:04

Update src/config/validate.ts

ad41274

Co-authored-by: Snir Shechter <me@snir.sh>

Update src/config/validate.ts

2278c4a

Co-authored-by: Snir Shechter <me@snir.sh>

shay123g mentioned this pull request Mar 23, 2021

complete Redis imp with ttl from config after #148 #164

Closed

Shay.Gazit added 2 commits March 23, 2021 21:17

.

ff7533a

fix imports

2cf1176

shay123g requested a review from SnirShechter March 23, 2021 19:32

Shay.Gazit added 3 commits March 28, 2021 19:37

Merge branch 'release/0.2.0' of https://github.com/mini-services/miniurl

0198804

into 16 � Conflicts: � src/config/types.ts � src/index.ts � src/services/storage/drivers/inMemory/index.ts � src/services/storage/drivers/relational/index.ts

fix tests

7c53866

fix tests

75c9a8f

shay123g requested a review from Danielg212 April 1, 2021 11:52

SnirShechter assigned Danielg212 May 14, 2021

url cleanup upgrade #148

Are you sure you want to change the base?

url cleanup upgrade #148

Uh oh!

Conversation

shay123g commented Mar 13, 2021

Uh oh!

codecov bot commented Mar 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

SnirShechter commented Mar 13, 2021

Uh oh!

shay123g commented Mar 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shay123g commented Mar 14, 2021

Uh oh!

Uh oh!

Uh oh!

lesagi left a comment

Choose a reason for hiding this comment

Uh oh!

shay123g commented Mar 16, 2021

Uh oh!

SnirShechter Mar 13, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SnirShechter Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

SnirShechter Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

SnirShechter Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

shay123g commented Mar 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Mar 13, 2021 •

edited

Loading

shay123g commented Mar 13, 2021 •

edited

Loading