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

New maintainer wanted #120

Open
agronholm opened this issue Jul 2, 2020 · 17 comments
Open

New maintainer wanted #120

agronholm opened this issue Jul 2, 2020 · 17 comments

Comments

@agronholm
Copy link
Owner

I find myself stretched too thin to effectively maintain this project. It would be nice if someone capable took the reins from me.

@flipbit03
Copy link

flipbit03 commented Jan 13, 2021

I want in.

I have a specific need (to have the created model descend from a baseclass like the example below

I want an option to be able to declare the base in a separate file the way I need, like :

@as_declarative()
class BaseProdigioPG:
    def __repr__(self):
        try:
            pkid = getattr(self, 'id')
            return f"<{self.__class__.__name__} PK_ID={pkid}>"
        except:
            return f"<{self.__class__.__name__} at {hex(id(self))}>"

Instead of :

# BaseProdigioPG = declarative_base()

This way one can easily customize the base class and add repr and whatever they need

....and I saw your (help wanted) issue.

How can we do this? I"ll try to provide the feature I want and get acquainted with the code. Let's see if we can make this work. This is a super useful tool and I thank you for your time/effort in doing this 👊

@agronholm
Copy link
Owner Author

The point of sqlacodegen was to save developers time they would otherwise have to spend writing the model classes themselves – a process that is both tedious and error prone.

I don't see anything that would prevent you from having a separate base class like this. All you have to do is edit the generated result.

At any rated, are you interested in maintainership or just getting this specific feature in?

@flipbit03
Copy link

flipbit03 commented Jan 15, 2021

I have interest in both mantaining the code (managing the pull requests and fixes) and also to have my own set of features in.

I have a use case where I have to query a PostgreSQL database which is being upstreamed somewhere else by other api, so I'm always lagging behind and have to update my base model according to the changes. I have also customized my base model and changed its name to something else (which is always desirable instead of "Base"). I'd like to have those kind of features baked in instead of having to edit the code every time i regenerate it. Simple quality of life features but which can go a long way.

I also found a bug where using a field name (namely: "text") triggers the generation of an invalid model since the next 'text' field inside a generated class which has a "text" column will actually use the text "Column" object instead of the 'text' function of SQLAlchemy, so I was about to dive/understand how the importcollector works and maybe I could extend it to detect when it needs to alias the SQLAlchemy objects because of name conflicts in the tables.

Because of those issues I found and the needs I have, I came here to check the pull requests/issues and maybe file a few of my own, and found your request for help. So how do we go about this? Can I just submit a first pull request so you can analyze the way I refactor/work with things so you can assess stuff first? that's probably what i'm gonna do.

Anyways, thanks in advance for this wonderful piece of software which has saved me countless hours.

@flipbit03
Copy link

flipbit03 commented Jan 15, 2021

All you have to do is edit the generated result.

For example, having a flag that means "insert THIS import file at the beginning of the file and don't create a base, instead, expect name to be the base (and create the generated classes using this object which will be defined elsewhere)" would mean one less manual edit. specially for us that have to 'sync' files from databases that are migrated/managed by other tools.

hope I can be of service!

@agronholm
Copy link
Owner Author

Other people have been trying to use sqlacodegen for similar workflows, even though it wasn't designed for that. But, if you're willing to take up maintainership, you can steer the project in that direction. We can start the way you suggested: make a PR or two and we'll go from there.

@agronholm agronholm pinned this issue Feb 7, 2021
@laurentS
Copy link
Collaborator

@agronholm I have a similar issue with a separate project that I started, which got more use than I expected (less than sqlacodegen, but that's not very relevant).
I got inspired by another project (I can't find it anymore, I wish I'd made a note of it) which took the route of adding every github member who contributed some code (PR, whatever you see fit) as a member of the project. Essentially this meant sharing the workload of reviewing PR, triaging bugs, etc...
I started doing this with the 6-7 contributors to my project a few weeks ago, so it's only very fresh, but overall, I'm seeing positive results: people review each other's PRs, start asking if they can cut a new release, etc...

Anyway, you might want to explore such an option as a possible path towards handing over maintainership of the project that is a bit less scary for other to jump into.

@agronholm
Copy link
Owner Author

The trio project at least did take that route. My only concern here is that this project could easily become unmaintainable when everyone adds their favorite customization hooks. Now with 3.0.0b2 out there, the need for a new maintainer is not that pressing. That said, I'm willing to experiment this if somebody wants to join the project as a contributor.

@anton-petrov
Copy link

Hi, @agronholm! Does the project still need new maintainers?
I recently tried it out and liked it. However, there are small bugs and roughness, but in general - sqlacodegen is a very indispensable thing. It's really awesome!

@agronholm
Copy link
Owner Author

Yes, I'd be interested in relinquishing maintainership after the next major release. Do you have any particular agenda for this project?

@leonarduschen
Copy link
Contributor

leonarduschen commented Apr 4, 2022

Hi, I would love to explore taking up maintainership for this project!

Although, I was hoping that could I work under the guidance of someone more experienced (both on this particular codebase and open source maintenance in general, mostly the later) for some time before fully committing.

@agronholm perhaps you could supervise my work for the next few weeks and see how it goes? I could work through the existing bugs in the issue tracker (well I've just opened a PR for one of them) and perhaps add some new features in.


One feature I think would be great to have is to allow customization of constraint naming conventions. We already have utils.uses_default_name, but it's currently not super useful because sqlalchemy.MetaData.naming_convention is always {'ix': 'ix_%(column_0_label)s'} by default. (Reference: https://docs.sqlalchemy.org/en/14/core/constraints.html#the-default-naming-convention)

We can pass the naming conventions as command line args like

sqlacodegen --conventions '"ix": "ix_%(column_0_label)s", "uq": "uq_%(table_name)s_%(column_0_name)s", "pk": "pk_%(table_name)s"'

or maybe have it read from a JSON file for easier formatting.

The implementation would be pretty straightforward. We just need to modify the MetaData in cli.main.

We will also need to make utils.uses_default_name check MetaData.naming_convention properly; right now it just checks for abbreviations: "fk", "pk", "ix", "ck", "uq" but the actual Constraint classes (e.g., PrimaryKeyConstraint) are valid as well. (Reference: https://docs.sqlalchemy.org/en/14/core/constraints.html#configuring-a-naming-convention-for-a-metadata-collection)

EDIT: formatting, typo

@agronholm
Copy link
Owner Author

@leonarduschen sounds good on the general level. I do question naming conventions being passed on the command line. That rabbit hole goes pretty deep when everybody wants to customize every aspect of the code generation step. A YAML or TOML based configuration file would be better. I'm looking forward to seeing what you come up with.

My intent with the next release is to create a system where you could have your own generator that just gets passed elements to be formatted (tables, classes, whatnot). If would then generate the code, optionally falling back to the default behavior.

@agronholm
Copy link
Owner Author

I've fixed all of the reported bugs so far so I've gone ahead and release v3.0.0rc1. I'm still not happy with the way import collection and rendering mesh together (or not) internally, so anybody taking on maintainership should find a better way to deal with that. @leonarduschen how about I pass the baton to you after 3.0.0 final?

@leonarduschen
Copy link
Contributor

Hi @agronholm , apologies for the prolonged silence, I've been wanting to get back to you on this


My intent with the next release is to create a system where you could have your own generator that just gets passed elements to be formatted (tables, classes, whatnot). If would then generate the code, optionally falling back to the default behavior.

Are you referring to the current "option" to subclass the existing generator classes as mentioned in https://github.com/agronholm/sqlacodegen#customizing-code-generation-logic ? Or is it something new that you wish to have in the next release (i.e., not 3.0.0) ?

I'm still not happy with the way import collection and rendering mesh together (or not) internally, so anybody taking on maintainership should find a better way to deal with that.

Well one area of improvement, I think, is how we do duplicated conditional checks for constraints on collect_imports_for_constraint and render_column. I'm guessing you're seeing something else on top of this - I'd be happy to hear your thoughts on this if you could elaborate on the details. Perhaps we should create a separate issue for the discussion.


@leonarduschen how about I pass the baton to you after 3.0.0 final?

I'm still not yet as comfortable with the codebase as I would ideally like myself to be, but if you'd still be participating in discussions from time to time I think we're good! I could definitely use your opinion on design decisions, possible features, etc - especailly early on.

@agronholm
Copy link
Owner Author

I think we need to support a configuration file to support all these options. They are getting more complex than can be reasonably supported as command line switches.

@agronholm
Copy link
Owner Author

Why the thumbs down?

@flipbit03
Copy link

I think a configuration file should only be considered after we finally find that something is difficult or impossible to do via the current configuration interface, which is the CLI.

I believe adding another interface and now having to coalesce from 2 sources of configuration inputs is extra work in documentation, testing and general wrangling, and that should only be pursued if it's an absolute necessity.

@agronholm
Copy link
Owner Author

The configuration file can certainly be deferred to a minor release after 3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants