mara / mara-app

A framework for distributing flask apps across separate packages with minimal dependencies
MIT License
15 stars 4 forks source link

do not modify the click command name when it is a MultiCommand #42 #44

Closed leo-schick closed 1 year ago

leo-schick commented 1 year ago

See #42

Supports implemeting nicer cli syntax e.g.

flask db migrate
flask pipeline run

# or with custom naming `mara`
mara db migrate
mara pipeline run

which is more intuitive than:

flask mara_db.migrate
flask mara_pipelines.ui.run
jankatins commented 1 year ago

Is this backwards compatible? The nice thing about having prefixes was that getting conflicts is a lot less likely, given that you have user contributed command and commands from libraries.

leo-schick commented 1 year ago

As long as you just add the new click.Group and don't throw out the current commands which are based on click.Command, it is backward compatible. I suggest to add a click.Group to all of the mara packages and throw out the old commands with the next major release of the package (e.g. for mara-db that would be version 5.0). This makes a migration smooth for those using the old commands in scripts.

I suggest to avoid conflicts by just using different command namespaces.

For example: Imagine I build my custom function which creates a database in PostgreSQL when the database does not yet exit. Exposing this would be typically in the mara-db package. The shell command should be flask db create <db_alias>. But since it is custom code, you would use your own namespace flask <my_company> db create <db_alias>.

An alternative approach would be merging the click groups by using click.CommandCollection. I have not looked into this. I currently suggest to use clear namespaces. I see a potential risk when someone experiments with external modules outside the mara suite which might use the same namespace. I guess this can be tackled when there is an case for this.

I strongly go for clear namespaces here. flask pipeline run or even mara pipeline run is 100% more intuitive as flask mara_pipelines.ui.run

leo-schick commented 1 year ago

@jankatins What do you think about putting this logic into a separate function so that it is patchable? Then people which want to use the old option still have the chance to do so.

leo-schick commented 1 year ago

@jankatins patchable function added