edgedb-python icon indicating copy to clipboard operation
edgedb-python copied to clipboard

Generate to a better default location with `--file` option

Open raddevon opened this issue 3 years ago • 7 comments

The code generator currently generates to the project root by default with the --file option if no path is provided. Changing the default path to dbschema would provide consistency with the JS code generator and would make it easier for users to be consistent with language best practice.

raddevon avatar Jan 17 '23 22:01 raddevon

We've had some discussion about this on Slack. @tailhook recommends an alternative default location for a single file:

I think it's probably best to calculate the common prefix. So if you have myapp/a/query1.edgeql and myapp/b/query2.edgeql they would be put in myapp/.

The rub here is that we can't guarantee modules in the common prefix are importable. Maybe there is a way to test for this and drop in an __init__.py if not?

raddevon avatar Jan 18 '23 18:01 raddevon

I'm a bit skeptical about adding a __init__.py file - it may bring unexpected results (like breaking implicit namespace packages as described in PEP 420). I think we tried our best to find the most probable place to put the single generated file (by looking at the common prefix), if it doesn't work - well, in the command line you can still specify where to put it, that should suffice?

fantix avatar Jan 18 '23 19:01 fantix

Another idea is to like just generate code into the standard output (when some non-default switch is turned on), and let the user decide where to put the file (and how to name it). This might've been discussed already, but as we're here ... 🤷‍♂️

fantix avatar Jan 18 '23 19:01 fantix

Are you saying we already look for the common prefix, @fantix? When I generated using --file, it was going to the project root even though all of my queries were in app/queries.

raddevon avatar Jan 18 '23 19:01 raddevon

Oh sorry I mean after fixing this issue, we will be "trying our best to use the common prefix", and probably don't need to add the __init__.py file.

fantix avatar Jan 18 '23 19:01 fantix

The impetus for this, in the event the context is helpful, was that @1st1 gave me feedback on the FastAPI example project that I shouldn't be importing a file from the root. I hadn't made the decision to do it that way consciously. I had just generated with edgedb-py --file, and that's where the file ended up. I asked if it makes sense for the generator to generate to a location the user shouldn't be importing a module from, and hence this issue was born.

All that to say, I'm not married to a particular fix for it if there is a better option than what's been proposed.

raddevon avatar Jan 18 '23 19:01 raddevon

If it's of any interest to you, what i do is store my .edgeql files in myappname/db/ and then run

poetry run -m edgedb.codegen --dir ./myappname/db/ --file ./myappname/db/__init__.py \
&& poetry run ruff format ./myappname/db/__init__.py

because i like being able to do

from myappname import db

db.query_name(db_client, a=b, ...)

MartinCura avatar Mar 22 '24 14:03 MartinCura