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

unattended_install: Remove the hack to get ks from providers #970

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ldoktor
Copy link
Contributor

@ldoktor ldoktor commented Mar 31, 2017

This hack was added in a144934 to allow
spice team to override the stock kickstart files, but it is actually a
backdoor which picks the first matching file from any provider, no
matter which provider is actually being used.

Let's disallow this behavior and ask people to submit their files to the
main repository.

@Andrei-Stepanov could you please take a look at this? I hope this backdoor is not needed anymore and this would be the target solution for me, anyway I'd be willing to temporarily change it to look for the file and if it does not exists look into providers files (basically change the order, first try to look in Avocado sources, then in backend dirs to allow adding extra files, but don't allow overriding existing Avocado files)

This hack was added in a144934 to allow
spice team to override the stock kickstart files, but it is actually a
backdoor which picks the first matching file from any provider, no
matter which provider is actually being used.

Let's disallow this behavior and ask people to submit their files to the
main repository.

Signed-off-by: Lukáš Doktor <[email protected]>
@ldoktor
Copy link
Contributor Author

ldoktor commented Sep 19, 2017

@Andrei-Stepanov, @clebergnu, @apahim, @will-Do would you please review this cleanup? It prevents from very unexpected and hard to debug issues.

@ldoktor
Copy link
Contributor Author

ldoktor commented Dec 11, 2017

Guys, especially @Andrei-Stepanov, @clebergnu, @apahim, @will-Do, would you please comment on this? It took me long time to understand what is going on and I don't want to go into this hell ever again.

@apahim
Copy link
Contributor

apahim commented Jan 19, 2018

@ldoktor, do the people using this know about the change? are they ready for this change?

@ldoktor
Copy link
Contributor Author

ldoktor commented Jan 20, 2018

I know @Andrei-Stepanov used to be using this, which lead to hard to debug issues and we ended-up fixing it by pushing their changes upstream and renaming their custom cfgs to not collide. This is a follow-up patch to get rid of this unexpected, hard-to-debug and really slow "feature" once for all but apparently no one cares.

After 10 months since the first notification and 4 months after the last one I think it's time to say they have nothing against.

@ldoktor
Copy link
Contributor Author

ldoktor commented Feb 16, 2018

@rdkdd @PandaWei @xutian @apahim @clebergnu @lmr would you please consider this for inclusion? It's been waiting for almost a year...

@rdkdd
Copy link
Contributor

rdkdd commented Feb 16, 2018

@ldoktor I have nothing against removing this bit.

@ldoktor
Copy link
Contributor Author

ldoktor commented Jan 22, 2019

@PandaWei @xutian @apahim @clebergnu @lmr any second reviewer for this thing? (I don't want to update it without knowing it's mergable...)

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

Successfully merging this pull request may close these issues.

3 participants