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

Generate to a better default location with --file option #410

Open
raddevon opened this issue Jan 17, 2023 · 7 comments
Open

Generate to a better default location with --file option #410

raddevon opened this issue Jan 17, 2023 · 7 comments
Assignees

Comments

@raddevon
Copy link
Contributor

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 raddevon changed the title Generate to dbschema by default with --file option Generate to a different location by default with --file option Jan 18, 2023
@raddevon
Copy link
Contributor Author

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 raddevon changed the title Generate to a different location by default with --file option Generate to a better default location with --file option Jan 18, 2023
@fantix
Copy link
Member

fantix commented Jan 18, 2023

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
Copy link
Member

fantix commented Jan 18, 2023

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 ... 🤷‍♂️

@raddevon
Copy link
Contributor Author

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.

@fantix
Copy link
Member

fantix commented Jan 18, 2023

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.

@raddevon
Copy link
Contributor Author

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.

@MartinCura
Copy link

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, ...)

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

No branches or pull requests

3 participants