Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SqlMagic Config File Locations #880

Closed
psychemedia opened this issue Sep 13, 2023 · 7 comments · Fixed by #899
Closed

SqlMagic Config File Locations #880

psychemedia opened this issue Sep 13, 2023 · 7 comments · Fixed by #899
Assignees

Comments

@psychemedia
Copy link

Currently, the database connection file defaults to ~/.jupysql/connections.ini, whereas magic settings are found in a pyproject.toml "in the current or parent directories" (docs).

I am trying to set up a clean environment for students to work in and would like to be able to hide the pyproject.toml file.

Ideally, it would be useful if the ~/.jupysql/ path were also a checked location for the SqlMagic confg settings.

@psychemedia psychemedia changed the title SqlMagic Config File SqlMagic Config File Locations Sep 13, 2023
@edublancas
Copy link

Yes, I think it makes sense to have a user-level configuration.

This seems relevant to decide what this location should be: psf/black#1899

I can ask someone from our team to work on this as soon as they finish their existing PRs, but it might take a couple of weeks (we'd highly appreciate it if you give us a hand)

When is your class starting?

@psychemedia
Copy link
Author

psychemedia commented Sep 13, 2023

Early students are dribbling in now, but we may end up using a legacy environment which uses ipython-sql for all manner of reasons (a planned migration straight from classic notebook to JupyterLab is blocked by most of the extensions we want to use not being available in JupyterLab 4. And we don't want to have to migrate twice (once to JL3, then to JL4).

I'm not sure how much help I can provide code-wise (I've not really engaged with tests, ever!)?

I do note that the file is loaded via:

file_path = util.find_path_from_root("pyproject.toml")

which leads to:

jupysql/src/sql/util.py

Lines 377 to 390 in a71ea40

def find_path_from_root(file_name):
"""
Recursively finds an absolute path to file_name starting
from current to root directory
"""
current = Path().resolve()
while not (current / file_name).exists():
if current == current.parent:
return None
current = current.parent
display.message(f"Found {file_name} from '{current}'")
return str(Path(current, file_name))

Currently, it looks like the connections file and path are picked up literally, so some thinking about how that path is declared may need rethinking.

jupysql/src/sql/magic.py

Lines 132 to 139 in 4011175

dsn_filename = Unicode(
default_value=str(Path("~/.jupysql/connections.ini").expanduser()),
config=True,
help="Path to DSN file. "
"When the first argument is of the form [section], "
"a sqlalchemy connection string is formed from the "
"matching section in the DSN file.",
)

The black repo you pointed to seems to use a find_project_toml() routine here.

@edublancas
Copy link

we need to modify this line:

file_path = util.find_path_from_root("pyproject.toml")

and replace it with some searching logic:

  • if ~/.jupysql/config.toml exists, use it
  • otherwise, look for a pyproject.toml in the current directory or parent directories

edge case: if both ~/.jupysql/config.toml and pyproject.toml, use pyproject.toml (?) - to allow users override settings

@psychemedia
Copy link
Author

In the edge case, and in case of conflict, should the ~/.jupysql/config.toml be prioritised for the jupysql settings, but the "root" pyproject.toml prioritised for the other settings?

@edublancas
Copy link

@marshallwhiteorg: let's see how other projects are tackling this (see my comment: #880 (comment)) and implement something similar

@marshallwhiteorg
Copy link

Sounds good, I'll implement this similar to black.

If pyproject.toml exists then that will be used, otherwise we'll look for ~/.jupysql/config.

@edublancas
Copy link

any updates @marshallwhiteorg ?

marshallwhiteorg added a commit to marshallwhiteorg/jupysql that referenced this issue Sep 27, 2023
marshallwhiteorg added a commit to marshallwhiteorg/jupysql that referenced this issue Oct 3, 2023
marshallwhiteorg added a commit to marshallwhiteorg/jupysql that referenced this issue Oct 4, 2023
marshallwhiteorg added a commit to marshallwhiteorg/jupysql that referenced this issue Oct 6, 2023
marshallwhiteorg added a commit to marshallwhiteorg/jupysql that referenced this issue Oct 6, 2023
marshallwhiteorg added a commit to marshallwhiteorg/jupysql that referenced this issue Oct 9, 2023
edublancas pushed a commit that referenced this issue Oct 10, 2023
* #880 User-level config

* 880 changelog

* 880 works if .jupysql dir doesn't exist yet

* #890 debugging windows

* #892 fix link

* Revert "#892 fix link"

This reverts commit 3354a8e.

* #890

* #880 fix link

* Revert "#880 fix link"

This reverts commit 815b05b.

* #880 PR feedback: tests and docs

* #880 formatting

* #880 patch -> monkeypatch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants