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.

Sign up for GitHub

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

Jump to bottom

replace /srv/tftpboot with config variable in cobbler.conf; variable … #2347

Closed
wants to merge 2 commits into from
Closed

replace /srv/tftpboot with config variable in cobbler.conf; variable … #2347

wants to merge 2 commits into from

Conversation

thiller2018
Copy link
Contributor

…is in setup.py

replace the static path /srv/tftpboot into a setup.py config variable.
The path for non-SUSE systems is taken from google search...

@thiller2018 thiller2018 mentioned this pull request May 27, 2020
Closed
@watologo1
Copy link
Contributor

Nice!
I try to come up with an on-top fix which passes tftproot from spec file to setup.py.
Be careful, the dirs we use in spec file are slightly different.
You also forgot /etc/cobbler/settings
Thanks for bringing this up with a nice starting patch!

@watologo1
Copy link
Contributor

Could you have a look at this one, please:
#2349

Be careful absolutely untested.
Your patch could then go on-top without the setup.py change.
And you need to add @@tftproot@@ to /etc/cobbler/settings as well (if this all works as expected).

This is all not nice, but at least variables are finally defined at one place (.spec)

SchoolGuy
SchoolGuy approved these changes May 29, 2020
View reviewed changes
Copy link
Member

@SchoolGuy SchoolGuy left a comment

Choose a reason for hiding this comment

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

LGTM with the current approach

@SchoolGuy SchoolGuy added this to Pull Requests in Backlog via automation May 29, 2020
@SchoolGuy SchoolGuy linked an issue May 29, 2020 that may be closed by this pull request
HTTPBoot not working on all non SUSE systems #2343
Closed
@SchoolGuy
Copy link
Member

@thiller2018 First of all: Thanks for your effort.

But: You deleted setup.py and you merged with upstream. I would prefer if you rebase and thus not delete setup.py.

@thiller2018
Copy link
Contributor Author

Fixed with commit "using environment variables" from June 5th.

@thiller2018 thiller2018 closed this Jun 8, 2020
@SchoolGuy SchoolGuy moved this from Pull Requests to Done in Backlog Jun 8, 2020
watologo1 added a commit to openSUSE/cobbler that referenced this pull request Jun 8, 2020
@watologo1
From: Thomas Hiller <thomas.hiller@hpe.com>
06061a7 
Make tftpboot directory dynamic and replace settings via setup.py

cobbler#2347
watologo1 pushed a commit to openSUSE/cobbler that referenced this pull request Jun 8, 2020
@thiller2018 @watologo1
Make tftpboot directory dynamic and replace settings via setup.py
6ff1d71 
cobbler#2347
@watologo1 watologo1 mentioned this pull request Jun 8, 2020
Merged
@watologo1
Copy link
Contributor

Fixed with commit "using environment variables" from June 5th.

Not really. I only added the setup.py part.
I now submitted this one:
#2359
And used your name/email @thiller2018 as the original patch was from you. Would be nice if you can have a quick look at it and verify that you are ok with it. Thanks!

@SchoolGuy SchoolGuy removed this from Done in Backlog Oct 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SchoolGuy SchoolGuy SchoolGuy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HTTPBoot not working on all non SUSE systems
3 participants
@thiller2018 @watologo1 @SchoolGuy