feat: abstract model storage and add csv output for walk command #316
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
Codecov Report
@@ Coverage Diff @@
## master #316 +/- ##
========================================
+ Coverage 44.9% 45.6% +0.6%
========================================
Files 20 22 +2
Lines 1978 2075 +97
========================================
+ Hits 890 947 +57
- Misses 969 997 +28
- Partials 119 131 +12 |
iand
changed the title
iand
force-pushed
the
iand/csv-output
branch
from
861dcc3 to
c34dfd4
Compare
iand
force-pushed
the
iand/csv-output
branch
from
c34dfd4 to
43fcf9f
Compare
frrist
approved these changes
Dec 14, 2020
Comment on lines
+751
to
+755
| if _, err := s.tx.ModelContext(ctx, m). | ||
| OnConflict("do nothing"). | ||
| Insert(); err != nil { | ||
| return xerrors.Errorf("persisting model: %w", err) | ||
| } |
There was a problem hiding this comment.
After this change lands would you expect #292 to be addressed here via reflection, via an interface the model implements, or something different?
There was a problem hiding this comment.
I think we could either handle in a few ways:
a) automatic via reflection magic (would be ideal but I'm unsure)
b) the model's Persist method could pass some hints when it calls PersistModel
c) we could add an UpdateModel method to StorageBatch that allows more control over the persistence
storage/csv_test.go
Outdated
| Message: "msg1", | ||
| } | ||
|
|
||
| dir, err := ioutil.TempDir("", "TestCSVPersist") |
There was a problem hiding this comment.
could use t.Name() as instead of TestCSVPersist
iand
force-pushed
the
iand/csv-output
branch
from
43fcf9f to
6e711ff
Compare
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.
Introduce
model.Storageandmodel.StorageBatchinterfaces to abstract away the details of the underlying model storage system.model.Storageis analogous to a database,model.StorageBatchis analogous to a transaction. A task should callStorage.PersistBatchwith a set of results to be persisted. The storage will call thePersistmethod on each result, passing in amodel.StorageBatch. A model should persist itself by callingStorageBatch.PersistModel. A composite type such as a list of results or a struct containing result fields should call thePersistmethod on each of its sub-results to ensure that they are persisted correctly.Replace all the direct references to
go-pgwith this pair of interfaces. Move a bunch of helpers to more appropriate packages.Add a
--csv=pathflag which will cause Visor to write data to csv files at that path, one file per database table. This flag overrides any database connection string if supplied. In other words Visor will not write to both a database and csv files.