You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Before the package gets adopted I would suggest thinking about a proper name.
There are 2 that needs to be thought of (I think)
First, the package name
from sapiclient import ... is imo not very descriptive
I'd suggest at least prefixing it with kbc or keboola, so something like:
kbcstorage (from kbcstorage import Client)
kbcsapiclient (from kbcsapiclient import Client)
kbcsapi (from kbcsapi import Client)
or something alike.
Second, the PyPi name (again $ pip install sapiclient doesn't feel right to me).
Or are we ok with installing directly from github $ pip install git+git://github.com/keboola/sapi-python-client.git as in https://github.com/keboola/python-docker-application?
This can be same as the (importable) package name, but can be different.
from keboola import StorageClient would be great aswell as having all keboola related stuff under one package. (Are there any other components which would benefit from Python bindings?)
imho, this client is mainly intended for external python applications to interact with the keboola storage api, rather than keboola python docker applications, as our docker layer handles the storage interface for us in those cases. (@odinuv do you agree with that assessment?)
for naming I really like from keboola import StorageClient
and I like pip install keboola-storage-client but am fine with installing directly via github too.
Just to be clear on one thing: for the from keboola import StorageClient to work we would have to either a) merge this with the python-docker-application, or b) rename the python-docker-application
I don' see any other way around this, since the python-docker-application already "owns" the keboola namespace
Edit:
Now I realized I used the term merge in a wrong way. It definitely wouldn't make sense to directly merge it into the docker app, but maybe create a brand new "keboola" app (and keep the old docker existing to not break existing apps) which would contain both the docker and the StorageClient.
Ok, let's start with installing from GitHub directly and not complicate things with PyPi
we definitely don't want to merge this into docker-application, they are two completely unrelated things. Sharing the namespace would be nice - but AFAIK, unless we want installing from pypi it's not a problem - i don't see a real difference between pip install git+git://github.com/keboola/sapi-python-client
and pip install keboola-storage-client
I used the term merge in a wrong way. It definitely wouldn't make sense to directly merge it into the docker app, but maybe create a brand new "keboola-python-bindings" app (and keep the old docker existing to not break existing apps) which would contain both the docker and the StorageClient.
If not, then I am strongly against naming this package keboola (therefore no from keboola import StorageClient). I think this would bring only confusion and what would you do if you wanted to have installed both?
come to think about it - it would be probably better to name the package kbc_storage - in the end it will be only storage API client (and they may be other clients, such as provisioning in future)
Good point, in that case we could rename the repo to keboola/python-client and then all keboola clients can live under a shared namespace of say keboola_client
I am still quite new to kbc so I am not aware of all components, so take me with a grain of salt. @odinuv do I understand you correctly, that the kbc api has multiple components
storage api for manipulating storage
metadata api for ...
components api for ..
and maybe sometimes in the future we'd like to develop py clients for all the components and you would prefer keeping the clients isolated is separate packages?
Honestly, I think it's more convinient (for both devs and users) to have them within one package
really let's keep it this to Storage API only, it itself is quite large. The other APIs do have completely different roles and even different authentication.
Regarding the packaging scope, it strikes me as a bridge that we can cross at a later date. Let's build this out for the storage api for now. If in the future there is another API that someone thinks should have a python client we can choose at that time to extend this package or make a different one.
As you wish guys, I'll send a PR (hopefully) this evening, I think I've got it almost ready.
I just wanted to avoid confusing the users down the road. It's simple to design it well now rather than renaming and merging later on when the code is used in production. I can see this screw up in case of Microsoft services (which was keeping me busy and frustrated last week). They have tons of different applications, APIs, platforms and versions. They keep renaming and moving them around and it's just so difficult to learn it and use it.
If something like that happens to us, I will perform a harakiri (most likely disembowel myself with a keyboard)
@pocin I'm keen to contribute to this but I don't want to duplicate work! Is there anything in particular that would be useful for me to look at until you make the pull request?
@Ogaday So sorry for the delay. I've made the PR along with a dev branch which now contains the refactored repo. I'd suggest checking it out, making PR's into it and once in a while we will merge it into the master.
@pivnicek because the master should be a "stable" branch (it's what you see when visiting https://github.com/keboola/sapi-python-client/) + when you install the package via pip install git+https://github.com/keboola/sapi-python-client) (I believe) the master branch is installed by default. The PRs we will be making are not necessary complete in the sense of the whole package. So I dont think its a good idea to pollute the master branch with only half-working stuff.
That's why I'd merge all PRs into the dev branch and merge the dev into master only once the dev branch is mature enough to be usable.
But again, this is the first time I am collaborating with multiple folks on a project so I can easily be wrong :)
imho master should be the lead, and tagged release versions should be used as stable, beta, etc. So, to get the current release it would be git+https://github.com/keboola/sapi-python-client:0.01
That way it stresses to keep the PRs/merges small enough to be manageable.
Let's not overengineer it. Any branch == unstable, master == stable. Whatever is stable goes to master - it does not have to be necessarily complete. The client won't support the complete API for long time (if ever), by far the most used features are creating and loading tables and creating buckets. Once we have those, we can have tag 1.0.0 . Then major features bump the version 1.1.0 etc. breaking changes bump the major version (though there shouldn't be much of them, the API is not changing). Whatever is in the master is the latest working (i.e. tests pass) and usable.
Sounds good to me. master branch <=> stable with versions controlled by git tags. #8 will dynamically track the git tag to provide version when calling the package.
Activity
pocin commentedon Jul 19, 2017
Before the package gets adopted I would suggest thinking about a proper name.
There are 2 that needs to be thought of (I think)
First, the package name
from sapiclient import ...is imo not very descriptiveI'd suggest at least prefixing it with kbc or keboola, so something like:
from kbcstorage import Client)from kbcsapiclient import Client)from kbcsapi import Client)or something alike.
Second, the PyPi name (again
$ pip install sapiclientdoesn't feel right to me).Or are we ok with installing directly from github
$ pip install git+git://github.com/keboola/sapi-python-client.gitas in https://github.com/keboola/python-docker-application?This can be same as the (importable) package name, but can be different.
How do you feel about
pip install keboola-storage-clientpip install kbc-storage-clientpip install kbc-sapi-clientOr any other suggestions?
On a related note - Does merging this sapi functionality with the https://github.com/keboola/python-docker-application makes sense?
from keboola import StorageClientwould be great aswell as having all keboola related stuff under one package. (Are there any other components which would benefit from Python bindings?)pivnicek commentedon Jul 19, 2017
I'll respond from bottom to top:)
imho, this client is mainly intended for external python applications to interact with the keboola storage api, rather than keboola python docker applications, as our docker layer handles the storage interface for us in those cases. (@odinuv do you agree with that assessment?)
for naming I really like
from keboola import StorageClientand I like
pip install keboola-storage-clientbut am fine with installing directly via github too.pocin commentedon Jul 19, 2017
Just to be clear on one thing: for the
from keboola import StorageClientto work we would have to either a) merge this with the python-docker-application, or b) rename the python-docker-applicationI don' see any other way around this, since the python-docker-application already "owns" the keboola namespace
Edit:
Now I realized I used the term merge in a wrong way. It definitely wouldn't make sense to directly merge it into the docker app, but maybe create a brand new "keboola" app (and keep the old docker existing to not break existing apps) which would contain both the docker and the StorageClient.
Ok, let's start with installing from GitHub directly and not complicate things with PyPi
pivnicek commentedon Jul 19, 2017
Ah ic, yes of course, that's too bad. That becomes a point in favour of the merge idea then.
odinuv commentedon Jul 19, 2017
we definitely don't want to merge this into docker-application, they are two completely unrelated things. Sharing the namespace would be nice - but AFAIK, unless we want installing from pypi it's not a problem - i don't see a real difference between
pip install git+git://github.com/keboola/sapi-python-clientand
pip install keboola-storage-clientpocin commentedon Jul 20, 2017
I used the term merge in a wrong way. It definitely wouldn't make sense to directly merge it into the docker app, but maybe create a brand new "keboola-python-bindings" app (and keep the old docker existing to not break existing apps) which would contain both the docker and the StorageClient.
If not, then I am strongly against naming this package
keboola(therefore nofrom keboola import StorageClient). I think this would bring only confusion and what would you do if you wanted to have installed both?odinuv commentedon Jul 20, 2017
so how about
kbc?pocin commentedon Jul 20, 2017
kbcsounds reasonable to meJust to make sure we are on the same boat:
https://github.com/keboola/sapi-python-client/pip install git+git:///github.com/keboola/sapi-python-client.gitAnd the directory structure will be something like this:
which will allow
from kbc import storagefrom kbc.storage import StorageClientand if we do
from .storage import StorageClientinsidesapi-python-client/kbc/__init__.pywe will be able to do justfrom kbc import StorageClientMakes sense? If so, I'll prepare a PR
pivnicek commentedon Jul 20, 2017
seems reasonable enough to me 👍 , with tests and package docs that would make
and at that point we can throw it on travis
odinuv commentedon Jul 20, 2017
wouldn't it be better to have just
client.py? So that we can havemetadata.pyandcomponents.pyin future like here https://github.com/keboola/storage-api-php-client/tree/master/src/Keboola/StorageApiodinuv commentedon Jul 20, 2017
come to think about it - it would be probably better to name the package
kbc_storage- in the end it will be only storage API client (and they may be other clients, such as provisioning in future)pivnicek commentedon Jul 20, 2017
Good point, in that case we could rename the repo to
keboola/python-clientand then all keboola clients can live under a shared namespace of saykeboola_clientodinuv commentedon Jul 20, 2017
why? the clients are unrealated and pypi doesn't have real namespaces anyway
pocin commentedon Jul 20, 2017
I am still quite new to kbc so I am not aware of all components, so take me with a grain of salt. @odinuv do I understand you correctly, that the kbc api has multiple components
and maybe sometimes in the future we'd like to develop py clients for all the components and you would prefer keeping the clients isolated is separate packages?
Honestly, I think it's more convinient (for both devs and users) to have them within one package
14 remaining items
odinuv commentedon Aug 14, 2017
really let's keep it this to Storage API only, it itself is quite large. The other APIs do have completely different roles and even different authentication.
pivnicek commentedon Aug 14, 2017
Welcome @Ogaday!
Regarding the packaging scope, it strikes me as a bridge that we can cross at a later date. Let's build this out for the storage api for now. If in the future there is another API that someone thinks should have a python client we can choose at that time to extend this package or make a different one.
pocin commentedon Aug 14, 2017
As you wish guys, I'll send a PR (hopefully) this evening, I think I've got it almost ready.
I just wanted to avoid confusing the users down the road. It's simple to design it well now rather than renaming and merging later on when the code is used in production. I can see this screw up in case of Microsoft services (which was keeping me busy and frustrated last week). They have tons of different applications, APIs, platforms and versions. They keep renaming and moving them around and it's just so difficult to learn it and use it.
If something like that happens to us, I will perform a harakiri (most likely disembowel myself with a keyboard)
odinuv commentedon Aug 15, 2017
Will try to keep that in mind, we don't want to loose you :)
Ogaday commentedon Aug 16, 2017
@pocin I'm keen to contribute to this but I don't want to duplicate work! Is there anything in particular that would be useful for me to look at until you make the pull request?
pocin commentedon Aug 16, 2017
@Ogaday So sorry for the delay. I've made the PR along with a
devbranch which now contains the refactored repo. I'd suggest checking it out, making PR's into it and once in a while we will merge it into the master.Let's decide what to do next in another issue #6?
Ogaday commentedon Aug 16, 2017
@pocin no worries at all! Sounds perfect to me.
pivnicek commentedon Aug 16, 2017
@pocin It's looking good! I'm a little confused though, why not make the PR for master?
pocin commentedon Aug 16, 2017
@pivnicek because the master should be a "stable" branch (it's what you see when visiting https://github.com/keboola/sapi-python-client/) + when you install the package via
pip install git+https://github.com/keboola/sapi-python-client) (I believe) the master branch is installed by default. The PRs we will be making are not necessary complete in the sense of the whole package. So I dont think its a good idea to pollute the master branch with only half-working stuff.That's why I'd merge all PRs into the
devbranch and merge thedevintomasteronly once thedevbranch is mature enough to be usable.But again, this is the first time I am collaborating with multiple folks on a project so I can easily be wrong :)
pivnicek commentedon Aug 16, 2017
Ah, I see where you're coming from.
imho master should be the lead, and tagged release versions should be used as
stable,beta, etc. So, to get the current release it would begit+https://github.com/keboola/sapi-python-client:0.01That way it stresses to keep the PRs/merges small enough to be manageable.
if you look at the releases tab here https://github.com/keboola/sapi-python-client/releases
Once we have
travisand the tests in place this will become automatic. For now though I think which ever way you are most comfortable with is fine.odinuv commentedon Aug 17, 2017
Let's not overengineer it. Any branch == unstable,
master== stable. Whatever is stable goes to master - it does not have to be necessarily complete. The client won't support the complete API for long time (if ever), by far the most used features are creating and loading tables and creating buckets. Once we have those, we can have tag 1.0.0 . Then major features bump the version 1.1.0 etc. breaking changes bump the major version (though there shouldn't be much of them, the API is not changing). Whatever is in the master is the latest working (i.e. tests pass) and usable.Ogaday commentedon Aug 17, 2017
Sounds good to me.
master branch <=> stablewith versions controlled by git tags. #8 will dynamically track the git tag to provide version when calling the package.odinuv commentedon Aug 17, 2017
y, I'll have a look at #8 in the afternoon
odinuv commentedon Aug 28, 2017
done in c32af27