feat: Add schema migration get and set cmds to CLI #1650
Conversation
AndrewSisley
added
feature
AndrewSisley
force-pushed
the
1648-lens-set-cli
branch
2 times, most recently
from
4771cfa to
baa9c8e
Compare
Codecov ReportPatch coverage:
@@ Coverage Diff @@
## develop #1650 +/- ##
===========================================
- Coverage 75.65% 74.89% -0.76%
===========================================
Files 200 203 +3
Lines 20807 21072 +265
===========================================
+ Hits 15740 15781 +41
- Misses 3990 4211 +221
- Partials 1077 1080 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
AndrewSisley
force-pushed
the
1648-lens-set-cli
branch
from
baa9c8e to
0270a85
Compare
AndrewSisley
changed the title
AndrewSisley
force-pushed
the
1648-lens-set-cli
branch
2 times, most recently
from
442e02e to
00dc5a6
Compare
AndrewSisley
changed the title
AndrewSisley
force-pushed
the
1648-lens-set-cli
branch
from
00dc5a6 to
5e4e203
Compare
AndrewSisley
force-pushed
the
1648-lens-set-cli
branch
from
5e4e203 to
e626774
Compare
There was a problem hiding this comment.
Approving with a todo and a couple suggestions.
Tests can be added later, possibly within this PR if I have time but do not count on it.
In the interest of getting this merged before your time off tomorrow and the code freeze, I can agree to this but I just want to put it out there that we should in the majority of cases avoid this delaying of test writing. We agreed to stay on top of test coverage and that means having tests that cover what is added/changed in the PRs.
| func MakeSchemaMigrationSetCommand(cfg *config.Config) *cobra.Command { | ||
| var lensFile string | ||
| var cmd = &cobra.Command{ | ||
| Use: "set [src] [dst] [cfg]", |
There was a problem hiding this comment.
thought (out-of-scope): We have a few different representations of args and flags. We should standardize the format #1651.
There was a problem hiding this comment.
I saw that ticket, it is a very good thing to do I think
cli/schema_migration_set.go
Outdated
| defradb client schema migration set bae123 bae456 '{"lenses": [...' | ||
|
|
||
| Example: set from file: | ||
| defradb client schema migration set bae123 bae456 -f schema_migration.graphql |
There was a problem hiding this comment.
thought: I find it odd that the file extension is graphql. Isn't it a json file that represents the config?
There was a problem hiding this comment.
Yes that is odd. I'll change it to something slightly less odd.
- change example file extension
cli/schema_migration_set.go
Outdated
| return errors.Wrap("join paths failed", err) | ||
| } | ||
|
|
||
| res, err := http.Post(endpoint.String(), "json", strings.NewReader(string(migrationCfgJson))) |
There was a problem hiding this comment.
todo: json isn't a valid content type. It should be application/json
There was a problem hiding this comment.
😁 my bad lol
-
application/json
100% agree with you |
## Relevant issue(s) Resolves sourcenetwork#1648 ## Description Adds `schema migration set` and `schema migration get` commands to the CLI, and the http API. Command/endpoint assumes the db server has access to the file path(s) provided in the lens cfg..
Relevant issue(s)
Resolves #1648
Description
Adds
schema migration setandschema migration getcommands to the CLI, and the http API.Command/endpoint assumes the db server has access to the file path(s) provided in the lens cfg - it will error early if it doesn't (tested manually).
I've done some manual testing of this and it looks all good. I'm happy merging this ASAP as time is now very short. Tests can be added later, possibly within this PR if I have time but do not count on it (#1652).