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

fix(service): better handling of script names #1951

Closed
wants to merge 2 commits into from

Conversation

imobachgs
Copy link
Contributor

Problem

There are a few problems when importing an AutoYaST script:

  • Agama uses file_name instead of filename.
  • If file_name is missing, it does not propose another name, generating an invalid profile.

Solution

Fix both problems by:

  • Using the right attribute name (filename instead of file_name).
  • Falling back to the basename from the location. If no location is given, propose script-X
    where X is a number.

Testing

  • Added a new unit test
  • Tested manually

@jreidinger
Copy link
Contributor

@imobachgs we really mess up with our amount of delayed PR as exactly this fixes I have in my PR #1946

@jreidinger
Copy link
Contributor

to be precise I solve it slightly differently, but I have those fixes and also test for it :)

@imobachgs
Copy link
Contributor Author

to be precise I solve it slightly differently, but I have those fixes and also test for it :)

Yes, I saw it later. Well, the main difference is that in this case I am trying to infer the name of the script from the URL if it is not given. But feel free to close it if you prefer the other way.

The only think I do not like about your PR is the "anonymous-" prefix. I would rather prefer something like "script-" or similar.

@imobachgs
Copy link
Contributor Author

Well, to reduce the duplication, I am closing this PR. I will review yours.

@imobachgs imobachgs closed this Jan 30, 2025
@jreidinger
Copy link
Contributor

thanks, change of fallback name is really trivial in mine...lets discuss it there.

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