completed changing max images to max storage #68
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oldmud0
reviewed
Nov 11, 2019
Comment on lines
191
to
+194
| id = (await db.run('INSERT INTO images DEFAULT VALUES')).lastID; | ||
| // Can we simplify this to just one insert statement? | ||
| await db.run('UPDATE images SET size = ? WHERE id = ?', | ||
| [metadata.size, id]); |
There was a problem hiding this comment.
Yes, it's possible to simplify into this:
INSERT INTO images(size) VALUES (?);The rest of the columns will be populated with their default values.
| await imageStore.setup(); | ||
|
|
||
| const ids = []; | ||
| for (let i = 0; i < 3; i++) { | ||
| ids[i] = await imageStore.addImage( | ||
| shapes[i], | ||
| imagery.Image.create({ time: i }) | ||
| imagery.Image.create({ time: i, size: 40}) |
There was a problem hiding this comment.
Suggested change
| imagery.Image.create({ time: i, size: 40}) | |
| imagery.Image.create({ time: i, size: 40 }) |
| * A limit can be placed on the maximum number of images stored by | ||
| * setting the maxImages parameter. When the limit is reached, | ||
| * A limit can be placed on the maximum size of images stored by | ||
| * setting the maxStorage parameter. When the limit is reached, |
There was a problem hiding this comment.
You should be absolutely clear on what the unit of maxStorage is supposed to be. I recommend megabytes.
| await this._withDb(async (db) => { | ||
| await db.run('CREATE TABLE IF NOT EXISTS ' + | ||
| 'images(id INTEGER PRIMARY KEY AUTOINCREMENT, ' + | ||
| 'deleted BOOLEAN DEFAULT FALSE)'); | ||
| 'deleted BOOLEAN DEFAULT FALSE,' + | ||
| 'size REAL)'); |
There was a problem hiding this comment.
size might want to be INTEGER since it's going to be in bytes.
| } else { | ||
| await db.run('INSERT INTO images(id) VALUES (?)', id); | ||
| await db.run('INSERT INTO images(id, size) VALUES (?, ?)', | ||
| [id, metadata.size]); |
There was a problem hiding this comment.
The metadata is currently not set up to pass in the size. You'll need to edit the base backend to calculate the size of each photo taken, and update the Protobuf message to include a size field.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.