-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
update to support s3 and avro serialization #18
Conversation
@lu-ko review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check all your catch
blocks and error handling.
src/lowstorage.ts
Outdated
}; | ||
|
||
const _hasColName = (colName: string = ''): void => { | ||
if (colName.trim() === '' || colName === null || typeof colName === 'undefined' || colName.length > 255 || colName === null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
colName === null
is checked twice.
src/lowstorage.ts
Outdated
this._schemas.set(colName, schema); | ||
} | ||
} catch (error) { | ||
throw new SchemaValidationError(`${MODULE_NAME}: Schema is invalid: ${schema}`, lowstorage_ERROR_CODES.SCHEMA_VALIDATION_ERROR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be difficult to troubleshoot cause of the problem, if error.message
is not logged/passed to SchemaValidationError
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And, maybe change SchemaValidationError
to lowstorageError
on this line, to reduce ${MODULE_NAME}:
chaining on line nr.241
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm yes plus if error class has constructor - there is no reason the provider module name on so many places for errors
fixed in version 2.0.0 |
#17 integrating pipelines for testing on minio + cloudflare
#13 compatibility w S3 (check https://github.com/sentienhq/ultralight-s3)
#11 adding option to enter some own props like https://github.com/good-lly/lowstorage/tree/v2?tab=readme-ov-file#constructoroptions-s3options / dirPrexif + access to s3