Fix notebook create on Windows #1996
Conversation
| @@ -65,3 +65,30 @@ def test_create(mock_create, runner): | |||
| notebook_name=FQN.from_string("my_notebook"), | |||
| notebook_file=notebook_file, | |||
| ) | |||
|
|
|||
|
|
|||
| @mock.patch("snowflake.connector.connect") | |||
There was a problem hiding this comment.
I've seen we have this exact patch repeated around 100 times in the codebase - wdyt about extracting it to a fixture? Requesting that fixture would still be repetitive, but at least we would not have to worry about proper ordering of mocks/args. If you think it makes sense, I'd be happy to take it as a warmup :)
There was a problem hiding this comment.
In testing entites we use constants to define those paths, however they repeat in every test file.
So +1 for any effort to go through tests, and create conts in test utils dir, to call them in such situation
| f"FROM '{stage_path}'\n" | ||
| "QUERY_WAREHOUSE = 'MockWarehouse'\n" | ||
| "MAIN_FILE = 'notebook.ipynb';\n" | ||
| "// Cannot use IDENTIFIER(...)\n" |
There was a problem hiding this comment.
This comment in query is intentional?
There was a problem hiding this comment.
TBH I don't remember - @sfc-gh-turbaszek git blames you for that :D
sfc-gh-pczajka
deleted the
SNOW-1878046-fix-notebook-create-on-windows
branch
* Add unit test * test more cases * fix stage path handling * update release notes
* Fix notebook create on Windows (#1996) * Add unit test * test more cases * fix stage path handling * update release notes * SNOW-1875316: Merge current connection context with config to preserve command line params (#2000) SNOW-1875316: Merge current connection context with config to preserve cmd line params * Fix issue with list of accounts in release channels and directives (#1997) * bump version * snapshot update --------- Co-authored-by: Marcin Raba <[email protected]> Co-authored-by: Michel El Nacouzi <[email protected]>
Pre-review checklist
Changes description
Fix notebook stage path handling on Windows