dolthub / doltpy

A Python API for Dolt
Apache License 2.0
56 stars 13 forks source link

core: add tag_create method #120

Open lucab opened 3 years ago

lucab commented 3 years ago

This adds a new tag_create method, using dolt tag CLI verb for tag creation.

oscarbatori commented 3 years ago

@lucab thanks for this contribution, would you mind naming the method tag instead of tag_create, as we are trying to stick closely to the command line interface in terms of naming convention?

lucab commented 3 years ago

@oscarbatori the problem with dolt tag is that conceptually it can do tag_list + tag_create + tag_delete in a single verb. I can rename this creation method to simply tag, but then I don't see how to add listing and deleting capabilities in the future on the Python side. Thoughts on how to proceed on that?

oscarbatori commented 3 years ago

@max-hoffman may have some views here, and he is going to be taking over Doltpy as dev lead. My general take is that we made a decision to replicate the CLI exactly in order to make it easy to make the transition. I think you correctly point out that this leads to some Python anti-patterns, though since they basically just pass commands into Popen calls I think the right call is the easy UX.

Our next release will ensure doltpy.cli is the only package with this kind of ugliness and we will expose a higher level API elsewhere that is more natural in Python.

So, I think for now we want to have this sort of ugly parameter pattern and then possibly expose something more Pythonic in doltpy.manage or whatever.

max-hoffman commented 3 years ago

Thanks @oscarbatori , and you bring up good points @lucab.

Generally we want to provide an interface that will be backwards compatible indefinitely. Pinning to MySQL and Git semantics is a good starting point. Identifying sections of dolt whose interfaces are likely to change with the progression of data science tools is also helpful.

For structuring the code, I am looking into how other libraries organize these sorts of interfaces:

If you have other comments, suggestions or ideas we'd be interested to hear. I am still getting up to speed, and refactoring the interface will be an on-going process.

lucab commented 3 years ago

The git API looks quite nice IMHO: a Tag class with a create class-method. However that would depart quite a bit from current API approach and will invert current relation (Tag::create(repo, name) vs repo.tag(name)).

I'm not in a hurry for this, so I'll be happy to let you settle on future API directions and the rework this the way you prefer.