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

[WIP] cephadm: add setup.py file for pip install #32662

Closed
wants to merge 1 commit into from

Conversation

Daniel-Pivonka
Copy link

add setup.py file for pip install of cephadm

Signed-off-by: Daniel-Pivonka dpivonka@redhat.com

Signed-off-by: Daniel-Pivonka <dpivonka@redhat.com>
@Daniel-Pivonka Daniel-Pivonka requested a review from a team as a code owner January 15, 2020 21:36
platforms='Linux',
packages=setuptools.find_packages(),
classifiers=[
"Programming Language :: Python :: 3",
Copy link
Contributor

@tchaikov tchaikov Jan 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cephadm is deliberately written so it's compatible to python2 and python3. better off putting

         "Programming Language :: Python :: 2",
         "Programming Language :: Python :: 3",

here.

@sebastian-philipp
Copy link
Contributor

Relates to #32526.

Problem here is, just adding a setup.py doesn't really solve anything:

  • how do we distribute it?
  • How do we intend to users to install cephadm'? sudo pip install 'git+https://github.com/ceph/ceph#egg=cephadm&subdirectory=src/cephadm'? Or upload to pypi?
  • How do we transfer cephadm in root mode to other hosts? The existing cat cephadm | ssh is dead-simple. In any case, this should IMO not install cephadm remotely.
  • how do we pass parameters and stdin to cephadm on remote machines?
  • Are there going to be any weird interactions when also installing cephadm via RPM?
  • CMake integration is missing
  • How do you want to distribute cephadm within the RPM package? as before or do you want to make it a proper RPM-packaged python package (like python-common)?

@tchaikov
Copy link
Contributor

yeah, after a second thought. pip install might not be a best way to redistribute cephadm..

@sebastian-philipp
Copy link
Contributor

sebastian-philipp commented Jan 16, 2020

yeah, after a second thought. pip install might not be a best way to redistribute cephadm..

Actually, I do think distributing via pip has many advantages that we should leverage, but we have to make it right and be sure to have answers for those questions.

e.g. Like doing pip install 'git+https://github.com/ceph/ceph#egg=cephadm&subdirectory=src/cephadm' looks more reasonable to me than just curling cephadm to a random directory.

Copy link
Contributor

@sebastian-philipp sebastian-philipp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a proper commit message. e.g. why do we want this change?

@jmolmo
Copy link
Member

jmolmo commented Jan 20, 2020

My opinion/answers about the @sebastian-philipp questions:

  • how do we distribute it?

Upstream: why not PyPi?
Downstream: In the case of Red Hat possibly one rpm.

The thing i would love to do would be to use only PyPi.... (no rpms)

  • How do we intend to users to install cephadm'? sudo pip install 'git+https://github.com/ceph/ceph#egg=cephadm&subdirectory=src/cephadm'? Or upload to pypi?

I think that using PyPi has advantages, for example, we can control the version provided and when it is updated the pacjkage in the index. A link to one file in one repository can have unexpected consequences. (the same unexpected consequences as if you install something contained in a file in some place in some repo using a curl command)

  • How do we transfer cephadm in root mode to other hosts? The existing cat cephadm | ssh is dead-simple. In any case, this should IMO not install cephadm remotely.

In upstream, if we use PyPi we do not need to transfer nothing.
In downstream, In the case of Red Hat we should have the rpm in the right repo.

  • how do we pass parameters and stdin to cephadm on remote machines?

Not understand very well. After cephadm is installed... what prevents you to execute commands remotely using ssh ?

  • Are there going to be any weird interactions when also installing cephadm via RPM?

I think that this is a downstream issue: It will depend basically of what things we "put" into cephadm, and the way to build the cephadm rpm. Do we want to have an rpm distribution now? maybe this should be part of another PR.

  • CMake integration is missing

Sorry , new with cephadm , and the only thing i have seen is an unique python file...probably missing something... do we need this integration?

  • How do you want to distribute cephadm within the RPM package? as before or do you want to make it a proper RPM-packaged python package (like python-common)?

I think that we should build an RPM only for "cephadm" with the required dependencies. The RPM used in Suse and RedHat is going to be the same?
I insist, i would love to install using only PyPi.

@sebastian-philipp sebastian-philipp changed the title cephadm: add setup.py file for pip install [WIP] cephadm: add setup.py file for pip install Feb 13, 2020
@stale
Copy link

stale bot commented Apr 13, 2020

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Apr 13, 2020
@sebastian-philipp
Copy link
Contributor

Daniel, can you rebase this?

@stale stale bot removed the stale label May 13, 2020
@Daniel-Pivonka
Copy link
Author

closing as this is not the solution to the problem. it may be part of it but it will need to be revisited in the future

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