Add saved query management functions.#191
Conversation
Linter didn't like:
```
> src/cb/saved_query.cr:95:29
> [W] Lint/NotNil: Avoid using `not_nil!`
> > name: @name.not_nil!,
```
So removed the `not_nil!`. It is now consistent with other
functions in the existing code base by not type-annotating
client method parameters
|
Tried a 3-phased approach to this PR with checkpoints that I could review. I also found that it made the process repeatable on a fresh branch.
Once artifacts were generated, started a new session and asked Claude to implement changes. Everything generated/passed on the first pass, save for the linter error. |
sfc-gh-abrightwell
left a comment
sfc-gh-abrightwell
left a comment
There was a problem hiding this comment.
LGTM. Just a few comments.
| query = client.get_saved_query query_id | ||
|
|
||
| filename = @file || "#{query.name.gsub(/[^a-zA-Z0-9_\-]/, "_")}.sql" | ||
| File.write(filename, query.sql) |
There was a problem hiding this comment.
Maybe have a rescue block in case this fails so that the user gets an error message over a stack trace?
|
|
||
| def run | ||
| validate | ||
| sql = File.read(@file.to_s) |
There was a problem hiding this comment.
Same here maybe, if the file doesn't exist?
Spitballing, but could maybe do something like:
unless File.exists?(@file.to_s) <some error>?
Maybe crystal does something reasonable though if not? Maybe worth validating? 🤷♂️
| end | ||
|
|
||
| it "exports to specified file" do | ||
| action.file = "/tmp/test_export.sql" |
There was a problem hiding this comment.
Might be worth considering using Dir.tempdir? As it abstracts the system tempdir which I think is /var/folders on macos and /tmp on Linux... 🤔
| CB::Model::SavedQuery.from_json resp.body | ||
| end | ||
|
|
||
| def create_saved_query(params) |
There was a problem hiding this comment.
It might be worth definite a param type for the create method. As I think that leaving it unbounded chokes spectator when generating the mock client. Or at least that's one thing I feel like eased the CI failures a little that I was debugging in #192.
struct SavedQueryCreateParams
include JSON::Serializable
property cluster_id : String?
property name : String?
property sql : String?
property? skip_enqueue : Bool = true
def initialize(@cluster_id, @name, @sql, @skip_enqueue = true)
end
end
def create_saved_query(params : SavedQueryCreateParams)
resp = post "saved-queries", params
CB::Model::SavedQuery.from_json resp.body
end
2 participants
No description provided.