add function to prune old files #8
Conversation
markysoft
changed the title
AzureDataService.js
Outdated
| @@ -28,6 +23,11 @@ class AzureDataService { | |||
| validateConfig(this); | |||
| } | |||
|
|
|||
| // eslint-disable-next-line class-methods-use-this | |||
There was a problem hiding this comment.
In order to avoid disabling eslint, if the format was set as a class level variable e.g. this.dateFormat = 'YYYYMMDD';, the variable could be used within the function e.g. return `-${startMoment.format(this.dateFormat)}.json`;. It could also be used in the following function and remove the duplication.
It might be worth moving the definition of dateFormat (should the suggestion above be followed) into lib/config.js so the other modules can access it.
There was a problem hiding this comment.
Good spot, I'll move the dateFormat to a class member first, then see where else 'YYYYMMDD' is used for further improvements
test/integration/azureDataService.js
Outdated
| await azureService.deleteFromAzure(containerName, `${azureDataService.seedIdFile}-20180314.json`); | ||
| }); | ||
|
|
||
| it('should remove data files older than date', async () => { |
There was a problem hiding this comment.
Given this test and the 2 following it execute the same function i.e pruneFilesOlderThan() and use some of the same functions to get the list of files in the blob. Could the setup be done for all 3 tests in one go and have the filters and expectations run separately? I know it is largely personal preference, the reason for changing would be to make it clear there is just one function being run that ends up prune all files.
It would also reduce the execution time.
There was a problem hiding this comment.
I think that makes sense, as that way there's a symmetry to the setup before and the teardown afterwards.
lib/filters.js
Outdated
| const regex = new RegExp(`^${outputFile}-(\\d{8})-${version}.json`); | ||
| return file => dateExpired(regex, file, oldestDate); | ||
| } | ||
| function createExpiredSummaryFilter(outputFile, summaryFile, version, oldestDate) { |
There was a problem hiding this comment.
A bit finickity but could the inconsistent spacing be fixed please? Ideally to have a line between each function.
No description provided.