feat: dem info endpoint (MAPCO-9814)#20
Conversation
|
🎫 Related Jira Issue: MAPCO-9814 |
asafmas-rnd
left a comment
asafmas-rnd
left a comment
There was a problem hiding this comment.
Looking good, keep up the good work!
asafmas-rnd
left a comment
asafmas-rnd
left a comment
There was a problem hiding this comment.
Looking Good, Thanks! 😎
Don't forget to fix the tests
* feat: additional validations * chore: pr comment restructure config * build: update to use temp config schema * feat: use config schema * chore: sync with boilerplate changes related to otel logging
| @injectAll('FileHandler') private readonly fileHandlers: FileHandler[] | ||
| ) {} | ||
|
|
||
| public async info(options: InfoOptions): Promise<InfoResponse> { |
There was a problem hiding this comment.
Why do we need both methods, "info" and "process"? As I see it, there is no logic in "info" class
There was a problem hiding this comment.
merged into single function
| try { | ||
| const { driver, format } = this.getDriver(filePath); | ||
| dataset = await driver.openAsync(fullFilePath, 'r'); | ||
| const band = await dataset.bands.getAsync(1); // DEMs are mostly single banded |
There was a problem hiding this comment.
"mostly"? How do we handle this use case?
There was a problem hiding this comment.
gdal-async will throw an error when accessing undefined band
since we access the first band the risk of an error thrown is negligible (zero bands raster is probably a very rare possibility)
| }; | ||
| } | ||
|
|
||
| private async validateMetadata({ band, dataset }: { band: RasterBand; dataset: Dataset }): Promise<void> { |
There was a problem hiding this comment.
here { band, dataset } and in line 72 https://github.com/MapColonies/dem-gateway/pull/20/changes#diff-f935e6e19b2f29c3c2718694e9a42775f3c2d4e23d48ff8db9eed98d87998a72R72 its { dataset , band } please be consistent
| }; | ||
| } | ||
|
|
||
| private async validateMetadata({ band, dataset }: { band: RasterBand; dataset: Dataset }): Promise<void> { |
There was a problem hiding this comment.
Why do you pass the arguments inside of an object and not as a method argument (several places)?
There was a problem hiding this comment.
I prefer it since it's best practice for long term maintainability and usage
| @@ -1,11 +1,11 @@ | |||
| { | |||
| "name": "service-name", | |||
| "name": "dem-gateway", | |||
There was a problem hiding this comment.
Update the service description
3 participants
missing: