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

Sle 15 sp3 #9

Merged
merged 28 commits into from
Aug 10, 2022
Merged

Sle 15 sp3 #9

merged 28 commits into from
Aug 10, 2022

Conversation

varkoly
Copy link
Collaborator

@varkoly varkoly commented May 22, 2022

Problem

If someone select an existing configuration the proposals will not read again. Unfortunately the proposal is saved into the configuration too. That is if the proposal was empty for the first time it will be empty next time too.
Furthermore I've detected that the modules in SLE15-SP4 ends with ".ko.zst".

Solution

After reading the selected configuration a new function refresh_all_proposals will be called.
I've changed the code for listing the modules in watchdog.rb tool. Now I'm using .gsub instead of File.basename

@coveralls
Copy link

coveralls commented May 22, 2022

Pull Request Test Coverage Report for Build 2832162981

  • 4 of 10 (40.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 29.504%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/clients/sap_ha.rb 0 1 0.0%
src/lib/sap_ha/configuration/watchdog.rb 1 2 50.0%
src/lib/sap_ha/configuration.rb 1 3 33.33%
src/lib/sap_ha/system/watchdog.rb 0 2 0.0%
Totals Coverage Status
Change from base Build 1573586455: -0.02%
Covered Lines: 1238
Relevant Lines: 4196

💛 - Coveralls

Peter Varkoly and others added 12 commits June 7, 2022 09:35
max lenngth 256
all ascii is allowed 33-126 instead of "*" and "/".
… from 33 until 125

expect of '*' and '/'. The identifier can be 256 character long
However, for security and technical reasons, we only allow alphanumeric characters as well as '-' and '_'.
The identifier must not be longer than 30 characters and it must be minimum 2 long.
Add some check for SID and site identifier.
Copy link

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

Just a few notes. Feel free to ignore all of them except the one about the Rakefile.

@@ -86,6 +86,13 @@ def initialize(role = :master)
@platform = SapHA::Helpers.platform_check
end

# Funktion to refresh the proposals of some modules. This is neccessary when

Choose a reason for hiding this comment

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

typo: function

@@ -235,8 +239,7 @@ def integer_in_range(value, low, high, message = '', field_name = '')
def sap_sid(value, message = '', field_name = '')
message = "A valid SAP System ID consists of three characters, starts with a letter, and "\
" must not collide with one of the reserved IDs" if message.nil? || message.empty?
flag = !SAP_SID_REGEXP.match(value).nil?
flag &= !RESERVED_SAP_SIDS.include?(value)
flag = !SAP_SID_REGEXP.match(value).nil? && !RESERVED_SAP_SIDS.include?(value)

Choose a reason for hiding this comment

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

what just flag = SAP_SID_REGEXP.match?(value) && !RESERVED_SAP_SIDS.include?(value)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be a boolean value.
With your syntax it delivers nil if the regexp does not match and braeks the testcase:
https://github.com/yast/yast-sap-ha/runs/7764069782?check_suite_focus=true#step:6:190

@@ -108,11 +108,13 @@ def installed_watchdogs

# Get the list of all watchdog available in the system
def list_watchdogs
wmods = []

Choose a reason for hiding this comment

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

np: I would say this initialization is not needed.

Rakefile Outdated
@@ -17,7 +17,7 @@

require "yast/rake"

Yast::Tasks.submit_to :sle15sp3
Yast::Tasks.submit_to :sle15

Choose a reason for hiding this comment

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

I would say this change is wrong. If you have the right version of yast-rake, then sle15sp3 should work.

@imobachgs imobachgs mentioned this pull request Aug 10, 2022
@varkoly varkoly merged commit ac154b8 into yast:SLE-15-SP3 Aug 10, 2022
@yast-bot
Copy link

yast-bot commented Nov 9, 2022

❌ Internal Jenkins job #1 failed

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.

None yet

5 participants