https://lakefs.io/ logo
Channels
Powered by
#dev
Title
# dev
v

Vaibhav Kumar

09/14/2022, 6:56 PM
Hi Team I am working on this issue https://github.com/treeverse/lakeFS/issues/2344 As far as I know I have to change the the README under the python package https://github.com/treeverse/lakeFS/tree/master/clients/python . Not sure why the changes doesn't reflect in pypy of lakefs already. Or if it is not how it was supposed to be done please guide me where to add the changes?
g

Guy Hardonag

09/14/2022, 7:27 PM
Hi @Vaibhav Kumar, Thanks for contributing sunglasses lakefs You are correct, It seems like the README is currently not being used for some reason, part of this issue is understanding whats not working there. I will give it a look tomorrow and help you out with the next steps.
v

Vaibhav Kumar

09/15/2022, 5:29 AM
So ideally the below read me should have displayed on pypy as well right? https://github.com/treeverse/lakeFS/blob/master/clients/python/README.md
g

Guy Hardonag

09/15/2022, 8:07 AM
Hi @Vaibhav Kumar, The description you see in pypi is taken from the 
long_description
field in the file https://github.com/treeverse/lakeFS/blob/master/clients/python/setup.py You should edit the long description, so it will contain the information requested in the issue. Just notice this file is generated using open-api ( you can’t just edit it) This data is generated in the Makefile (
client-python: api/swagger.yml  ## Generate SDK for Python client
)
v

Vaibhav Kumar

09/15/2022, 12:36 PM
I need to edit this README and there will be couples of code line changes in Setup.py . Please confirm if this approach is ok with you?
g

Guy Hardonag

09/15/2022, 1:15 PM
Unfortunately that won’t be enough, you would need to edit the makefile, in a way that when running 
make client-python
the 
long_description
in 
Setup.py
will include a link to the 
pydocs
.
v

Vaibhav Kumar

09/15/2022, 6:52 PM
Whatever we see on the pypy site is from the text present in the 
long_description
and so just changing the setup and recompiling should work
What I have read from internet is you just need to edit setup.py, recompile your makefile and that should be enough. I didn't see it anywhere to edit the makefile. If you have any doc to refer can you please share?
g

Guy Hardonag

09/15/2022, 8:36 PM
The python client is automatically generated every time we change the api If you just change the 
setup.py
file, the next time the python client will be generated your changes will be overwritten. You could mimic it running the following steps: 1, make a change in 
setup.py
2. Run 
make client-python
from the root of the project 3. go back to 
setup.py
- your changes where removed your change should still exist after running 
make client-python
The python client is built in the Makefile, using the 
openapi-generator
you could read about it more here
v

Vaibhav Kumar

09/16/2022, 6:57 PM
where are we uploading the python distribution package to pypi , I couldn’t find it in 
Makefile
or 
swagger.yml
@Guy Hardonag
g

Guy Hardonag

09/16/2022, 7:03 PM
We don't, In the makefile we run make client-python That command generates the python client code And the setup.py file
v

Vaibhav Kumar

09/16/2022, 7:06 PM
ok, then how do you put the python lakefs package to pypi repository ? I am a bit confused
g

Guy Hardonag

09/16/2022, 7:16 PM
If you would like to see how that happens you could look at https://github.com/treeverse/lakeFS/blob/master/.github/workflows/python-api-client.yaml This workflow builds (using the makefile) and publishes (to pypi ) the python client.
v

Vaibhav Kumar

09/16/2022, 8:38 PM
I have tried putting a lakefs readme on a sample package on pypi test and it is working fine. I Have uploaded my package using twine. As you said earlier`make client-python` has replaced my code, now I am lost what and where I am supposed to change to avoid this overwritten behaviour . Kindly help.
g

Guy Hardonag

09/17/2022, 3:12 AM
Looks good 👍 If you’d like, we could meet and talk about it on Sunday or Monday.
v

Vaibhav Kumar

09/17/2022, 3:56 AM
Sure we can, Sunday works for me
Please let suggest sometime
👍 1
g

Guy Hardonag

09/17/2022, 3:59 AM
Moving to a private chat
v

Vaibhav Kumar

09/19/2022, 5:53 AM
Hi @Guy Hardonag I tried my best to find the README param in open API but I didn’t find any such thing. Could you please help?
g

Guy Hardonag

09/19/2022, 5:56 AM
Hi @Vaibhav Kumar , thanks! looks like we will need to add a script that will edit the 
setup.py
file after the python client is generated. I will look into it today and get back to you by the end of the day
👍 1
v

Vaibhav Kumar

09/19/2022, 6:01 AM
So whatever changes I did in setup.py in the forked repo are those needed as of now?
g

Guy Hardonag

09/19/2022, 7:01 AM
Yes, we will need to write a script that injects your changes
Hi @Vaibhav Kumar , I’m sorry, didn't get to it today I’m going on vacation tomorrow. @Itai David will help you out.
v

Vaibhav Kumar

09/19/2022, 5:55 PM
Sure, @Guy Hardonag enjoy your vacation
🙏 1
@Itai David I hope you are aware about the issue, let me know if you need any details
i

Itai David

09/19/2022, 5:58 PM
Hi @Vaibhav Kumar @Guy Hardonag already filled me in on the details.
v

Vaibhav Kumar

09/19/2022, 6:04 PM
Ok, so how shall we proceed then?
i

Itai David

09/19/2022, 6:16 PM
As far as I understood, I will need to update our build process - probably add a script - to add your work to 
setup.py
, after the client is generated. Did I get this correctly?
v

Vaibhav Kumar

09/19/2022, 6:20 PM
Yes correct
i

Itai David

09/19/2022, 6:23 PM
I will have to dig in a bit, in order to find out how to do that, and get back to you with that. OK?
v

Vaibhav Kumar

09/19/2022, 6:27 PM
Ok
🙂 1
i

Itai David

09/20/2022, 4:13 AM
Hi @Vaibhav Kumar I haven't completed this one yet. I will put some more effort tomorrow (my tomorrow 🙂 ) and get back to you with an update Thank you
v

Vaibhav Kumar

09/20/2022, 4:28 AM
Works for me, once my changes are deployed I will pick up the next issue ☺️
👍 1
@Itai David
Hi @Itai David Any updates?
i

Itai David

09/21/2022, 1:04 PM
Hi @Vaibhav Kumar. I'm still trying to figure this one out. There seem to be a straight-forward way to set this update, using ppenapi generator arguments, but for some reason it does not work and openapi generator ignores it. I'm trying to figure out if this is a bug that can be solved easily (e.g. updating openapi generator etc.) but so far with no success Will update you with any progress
v

Vaibhav Kumar

09/21/2022, 1:10 PM
What all arguments you think should work?
i

Itai David

09/21/2022, 2:11 PM
I was looking at the 
--additional-properties
flag. Looking at the code it seems it should allow an override with 
appDescription
key, but looks like it is ignored
v

Vaibhav Kumar

09/21/2022, 2:26 PM
Yes , @Guy Hardonag also tried it but didn't work. So he said about making the script
i

Itai David

09/21/2022, 2:35 PM
Yes. The script he was referring to is a bit of a hack. I'm trying to see if there's a more straight forward way to achieve the same result. Currently looking on how to use user-defined templates
v

Vaibhav Kumar

09/21/2022, 3:05 PM
Ok , please keep me posted for any updates
Meanwhile what else shall I pick considering I know python and java
i

Itai David

09/22/2022, 12:56 AM
Sorry for the delay - I somehow forgot to hit the send button... We have this one: https://github.com/treeverse/lakeFS/issues/2801. It involves another client - the hadoopFS client and will require some 
lakeFS
work (bringing a server up, configuring the client etc.) - we should have all the required info documented, with most of it as a step-by-step guides). It will probably need some 
spark
work too - in case you are up to it
Hi @Vaibhav Kumar. Great update: I have https://github.com/treeverse/lakeFS/pull/4230 in review. It adds a user-defined template to generate 
setup.py
. It is currently identical to the one used by OpenAPI Generator, but you can edit the 
long_description
string to fit your needs. Feel free to branch-out from my branch https://github.com/treeverse/lakeFS/tree/2344-pyhton-client-project-description, or to merge it to yours. Good luck
v

Vaibhav Kumar

09/22/2022, 4:26 AM
@Itai David Ok, So now I can pull from https://github.com/treeverse/lakeFS/tree/2344-pyhton-client-project-description and just add my changes and then give you the fork right?
i

Itai David

09/22/2022, 10:55 AM
Hi @Vaibhav Kumar - Once you are done with your work you can open a PR
v

Vaibhav Kumar

09/22/2022, 12:55 PM
@Itai David if I am applying my changes and and then using 
make client-python
is replacing my changes. I have pulled your branch to apply these changes on.
@Itai David https://github.com/treeverse/lakeFS/pull/4241 Please take a look
👀 1
I have raised the PR
i

Itai David

09/22/2022, 3:11 PM
Did you try updating the 
setup.mustache
template file I added. The generator creates 
setup.py
based on that template. The one I submitted is the default one. I think that be adding your changes to 
setup.mustache
will have them reflected in 
setup.py
. Can you give it a go?
v

Vaibhav Kumar

09/26/2022, 11:38 AM
@Itai David I have put my changes to 
setup.mustache
but if I clone from your branch I am not able to raise a PR. I get permission denied, I am not able to create multiple forks as well.
i

Itai David

09/26/2022, 1:20 PM
Hi @Vaibhav Kumar I'm not really sure why is that. Can you try and merge my branch into yours, or rebase your branch from mine. I think it will add my commit to your branch and PR, and will let you merge it all together
v

Vaibhav Kumar

09/27/2022, 5:02 PM
Hi @Itai David I have merged your branch to mine and raised the below PR https://github.com/treeverse/lakeFS/pull/4260
Let me know if you have any queries.
i

Itai David

09/27/2022, 5:12 PM
Hi @Vaibhav Kumar. Thank you for that. Let me take a look at it 🙂
Hi Again @Vaibhav Kumar, and thanks again for this contribution. I reviewed the code and it looks great. However, I'm not a part of the team usually handling the clients code, so I would like it to be reviewed by that team. As today is a holiday in Israel, they will probably only be available to review it tomorrow. I will keep you posted
v

Vaibhav Kumar

09/27/2022, 7:48 PM
That is fine @Itai David
I was going through the checks and seems like one check 
Test / Run Linters and Checkers (pull_request)
has failed and rest has passed, is that fine?
i

Itai David

09/27/2022, 9:33 PM
Hi @Vaibhav Kumar - that's something that needs to be addressed. It verifies that the newly generated client matches the submitted code. I think that generating the client based on your recent version, and adding the generated code to the PR, should fix that. Can you please give it a try?
v

Vaibhav Kumar

09/28/2022, 11:57 AM
I ran 
make client-python
again which auto generated code to setup.py with my changes which I had put in 
setup.mustache
. Is that what you were asking for? @Itai David
i

Itai David

09/28/2022, 12:57 PM
Yes please. We need the committed 
setup.py
to match the one that will be generated by the build Thank you @Vaibhav Kumar
v

Vaibhav Kumar

09/28/2022, 12:58 PM
Done updated my PR
👀 1
i

Itai David

09/28/2022, 1:00 PM
Looks great. I will wait for the checks to complete and ping the relevant team to prioritize the review
I pinged them. Will keep you posted 🙂
10 Views
Powered by