[Spacewalk-devel] Cleanup of StringResource in java

2012-03-05 Thread Miroslav Suchy

On 5.3.2012 11:10, Tomas Lestach wrote:

>  Is the removal of strings with this file as context needed?

That would be really nice, even if we usually do not remove the unused
strings.:(



Challenge accepted.

I run:
git grep '| sed 's/\.plural//' | grep -v metrics\\\. | grep -v 'user prefix '| 
grep -v 'kickstart.jsp.virt-type' | grep -v 'address type ' | grep -v 
'enMessage' | sort | uniq | while read id; do git grep "$id" .. | grep 
-v StringResource | grep "$id" >/dev/null || echo $id; done >/tmp/o


cat /tmp/o | while read id; do git grep -l "$id" | while read file; do 
sed -i "//d" "$file"; done; git 
commit -a -m "removing unused string with trans-id '$id'"; done


I then reviewed it and after I removed some false negatives, I commited 
it as one big commit.


That commit is huge and in this volume, I'm sure I removed some string, 
which are created by concatenating strings in code. I will provide list 
of trans-unit I removed at the bottom of this email.

I have all those individual commits in my private branch.
So if you will notice some untranslated string in WebUI (it is always 
displayd as **name-of-trans-unit**), please ping me and I will revert 
only that one string. Also if you notice in this list of some trans-unit 
you seen recently and you know it is in use, please let me know.


Mirek

List of removed trans-unit:
actions.jsp.scheduledaction
actions.none
activation-key.jsp.header2
addfiles.jsp.create-link
addfiles.jsp.header2
addfiles.jsp.import-link
addfiles.jsp.upload.jspf.title
addresses.SendVerification
address1.displayname
affectedsystems.jsp.management
affectedsystems.jsp.provisioning
api.kickstart.iprange.invalid
api.kickstart.labelalreadyinuse
api.package.provider.duplicateKey
api.system.invalidarch
assignedgroups.jsp.accessible
assignedgroups.jsp.header2
audit.nav.audit
bootstrap.config.error.10
bootstrap.config.error.11
bootstrap.config.error.13
bootstrap.config.error.14
bootstrap.config.error.15
bootstrap.config.error.16
bootstrap.config.error.17
bugzilla.jsp.submitbug
bunch.jsp.description.cleanup-data-bunch
bunch.jsp.description.clear-taskologs-bunch
bunch.jsp.description.cobbler-sync-bunch
bunch.jsp.description.compare-configs-bunch
bunch.jsp.description.daily-status-bunch
bunch.jsp.description.errata-cache-bunch
bunch.jsp.description.errata-queue-bunch
bunch.jsp.description.channel-repodata-bunch
bunch.jsp.description.kickstart-cleanup-bunch
bunch.jsp.description.kickstartfile-sync-bunch
bunch.jsp.description.package-cleanup-bunch
bunch.jsp.description.sandbox-cleanup-bunch
bunch.jsp.description.satcert-check-bunch
bunch.jsp.description.session-cleanup-bunch
bunch.jsp.description.sync-probe-bunch
Buy Now
cancel.displayname
Certificate Administrators
certificate.config.error.10
certificate.config.error.11
certificate.config.error.20
certificate.config.error.30
certificate.config.error.40
certificate.config.error.80
certificate.config.error.82
certificate.config.error.83
certificate.config.error.84
certificate.config.error.85
certificate.config.error.86
certificate.config.error.87
city.displayname
cloneerrata.jsp.yes
common.download.alert
common.download.header
common.download.channel-name
common.download.no-isos
common.download.not-sure
common.download.select
common.download.select-or-all
company.displayname
companyname.displayname
config.common.edit
config.common.noSystems
config.common.quota
config.common.quotaAlt
config.common.sysconfigAlt
config.error.path-required
configfilefilter.path
config-file-form.error.end-delim-percent
config-file-form.error.group-invalid
config-file-form.error.mode-invalid
config-file-form.error.start-delim-percent
config-file-form.error.user-invalid
config_files.setFilesToImport.success
config_files.setFilesToRemove.failure
config_files.setFilesToRemove.success
config_files.unsubscribeSystems.success
config.files_0_dirs_0_symlinks_0
config.files_0_dirs_0_symlinks_0_url
config.files_0_dirs_0_symlinks_1
config.files_0_dirs_0_symlinks_2
config.files_0_dirs_1_symlinks_0
config.files_0_dirs_1_symlinks_1
config.files_0_dirs_1_symlinks_2
config.files_0_dirs_2_symlinks_0
config.files_0_dirs_2_symlinks_1
config.files_0_dirs_2_symlinks_2
config.files_1_dirs_0_symlinks_0
config.files_1_dirs_0_symlinks_1
config.files_1_dirs_0_symlinks_2
config.files_1_dirs_1_symlinks_0
config.files_1_dirs_1_symlinks_1
config.files_1_dirs_1_symlinks_2
config.files_1_dirs_2_symlinks_0
config.files_1_dirs_2_symlinks_1
config.files_1_dirs_2_symlinks_2
config.files_2_dirs_0_symlinks_0
config.files_2_dirs_0_symlinks_1
config.files_2_dirs_0_symlinks_2
config.files_2_dirs_1_symlinks_0
config.files_2_dirs_1_symlinks_1
config.files_2_dirs_1_symlinks_2
config.files_2_dirs_2_symlinks_0
config.files_2_dirs_2_symlinks_1
config.files_2_dirs_2_symlinks_2
configchannelfilter.name
config_channel.local_override
config_channel.normal
config_channel.server_import
config_channels.failure
config_channels.success
config_channels_to_unsubscribe.unsubscribe.failure
config.channels_0
config.channe

Re: [Spacewalk-devel] [PATCH] Bug about cloned channels and errata

2012-03-05 Thread Johannes Renner
On 03/05/2012 03:40 PM, Tomas Lestach wrote:
> On Friday 02 of March 2012 18:11:41 Johannes Renner wrote:
>> On 02/29/2012 05:38 PM, Cliff Perry wrote:
>>> On 02/29/2012 11:05 AM, Johannes Renner wrote:
 Hello,

 I am investigating into a bug about cloned channels and errata. This
 is how to reproduce it on the UI:

 1. Clone a channel that contains errata, but select "clone without
 errata" 2. Go to Channels -> Manage Software Channels -> Errata ->
 Add -> Add Custom Errata>> 
(or Add RedHat Errata) -> Unmark the checkbox "Package Assoc."
if necessary>> 
 3. Choose an Erratum, clone it and confirm

 Result: The cloned erratum can be found in the cloned channel as
 "CLxxx"

 5. Now subscribe a system to the newly cloned channel only
 6. Go to this System -> Software -> Errata

 Result: There is two errata listed, "xxx" and "CLxxx", but only
 "CLxxx" should be. Also the table 'RhnServerNeededCache' shows even
 more entries where errata_id == null?

 I found a lot of open bugs about this topic, but not exactly this one.
 If one of you can reproduce it I could create a bug report for
 spacewalk, or are you aware of such misbehavior already?

 To me it looks like the used statement
 ('insert_new_cache_entries_by_packages' in ErrataCache_queries.xml)
 is not doing the right thing. There even seems to be an easy fix that
 I attached as a patch for master, which just calls a stored procedure
 ('update_needed_cache_for_channel', ErrataCache_queries.xml) instead.

 Maybe someone of you who knows about this code could have a look at
 the issue, I might be missing something ..
>>>
>>> Replying to add more background. I was not aware of this specific bug
>>> myself either. I'd be interested to see if we can reproduce on Sat 5.4
>>> code as well as current Spacewalk code.
>>>
>>> This area of the code seems to get updates and re-factor once every
>>> couple of years, due to subtle bugs in logic, along with
>>> scalability/performance feedback.
>>>
>>> The particular line you attached to propose changing was last changed in
>>> Feb 2009 as part of a larger re-factor:
>>> http://git.fedorahosted.org/git/?p=spacewalk.git;a=commitdiff;h=3707af41
>>> 49d84a1ddb1ce693c78f7b791a044cd1
>>>
>>> If memory serves me correct, we were in the middle of a Satellite
>>> release cycle, so changing the stored procedure was not an ideal
>>> solution for Satellite. We fixed Java code - which performed well.
>>>
>>>  I'm not sure if I'd want the fix, as proposed, in 1.7, without us
>>>
>>> spending time to confirm it is the best solution for logic and
>>> performance. I don't think we will have time to dedicate to assist until
>>> post 1.7. If one the guys finds we have time to dig in before 1.7 goes
>>> out in the next week (hopefully) - we will.
>>>
>>> Cliff
>>
>> Sorry, I now found out I was experiencing this bug and was missing the
>> patch:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=707658
>>
>> So the cloned patches now show up correctly on the UI, but anyways I think
>> there is some code in here that does strange things. E.g. when I analyze the
>> file PublishErrataAction.java:
>>
>> The newly cloned patch is published by calling in line 80:
>>
>> ErrataManager.publishErrataToChannelAsync(...)
>>
>> This also triggers insertion into the cache table ("rhnServerNeededCache").
>> So why do we need to call later on (and without the actual errata ids):
>>
>> ErrataCacheManager.insertCacheForChannelPackagesAsync(chanList, pidList);
>> (line 100)
>>
>> This seems to insert another row into rhnServerNeededCache where the eid is
>> null! If this is wanted behaviour, then what is it for?
> 
> Right Johannes,
> 
> good catch.
> this definitelly isn't wanted behavior.
> If the added packages to a channals are associated with any erratum (what is 
> our case), we definitelly want to consider also the erratum, when 
> regenerating 
> errata cache.
> 
> Will you find some time to verify the behavior, when removing the errata 
> cache 
> update on line 100?
> If not, please, file a BZ for the issue.

I will find time for that in the next days and I can then come up with a new
patch for that file after I verified the behavior.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] rhn-prefix check when creating new channel

2012-03-05 Thread Duncan Mac-Vicar P.

On 03/02/2012 04:20 PM, Miroslav Suchý wrote:

On 03/02/2012 02:57 PM, Duncan Mac-Vicar P. wrote:

 As we replaced
some errror messages "Redhat" with a configurable vendor string
we now get the error message as


Did I miss this patch?


- Or its generic version: make it a "vendor restriction" and the regex
configurable


Hmm, this one sounds best to me.


For channel validation there seems to be a bunch of duplicated code: 
NewChannelHelper and CreateChannelCommand both perform very similar 
validations.


If I make the regexp "rhn" independent, configurable and 
switchable/optional it would be easier if the validations are refactored 
in one place.


I think the right place for the validations is the helper, and the 
command class should consume the validations from the helper. Is that fine?


Duncan






___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] [PATCH] Bug about cloned channels and errata

2012-03-05 Thread Tomas Lestach
On Friday 02 of March 2012 18:11:41 Johannes Renner wrote:
> On 02/29/2012 05:38 PM, Cliff Perry wrote:
> > On 02/29/2012 11:05 AM, Johannes Renner wrote:
> >> Hello,
> >> 
> >> I am investigating into a bug about cloned channels and errata. This
> >> is how to reproduce it on the UI:
> >> 
> >> 1. Clone a channel that contains errata, but select "clone without
> >> errata" 2. Go to Channels -> Manage Software Channels -> Errata ->
> >> Add -> Add Custom Errata>> 
> >>(or Add RedHat Errata) -> Unmark the checkbox "Package Assoc."
> >>if necessary>> 
> >> 3. Choose an Erratum, clone it and confirm
> >> 
> >> Result: The cloned erratum can be found in the cloned channel as
> >> "CLxxx"
> >> 
> >> 5. Now subscribe a system to the newly cloned channel only
> >> 6. Go to this System -> Software -> Errata
> >> 
> >> Result: There is two errata listed, "xxx" and "CLxxx", but only
> >> "CLxxx" should be. Also the table 'RhnServerNeededCache' shows even
> >> more entries where errata_id == null?
> >> 
> >> I found a lot of open bugs about this topic, but not exactly this one.
> >> If one of you can reproduce it I could create a bug report for
> >> spacewalk, or are you aware of such misbehavior already?
> >> 
> >> To me it looks like the used statement
> >> ('insert_new_cache_entries_by_packages' in ErrataCache_queries.xml)
> >> is not doing the right thing. There even seems to be an easy fix that
> >> I attached as a patch for master, which just calls a stored procedure
> >> ('update_needed_cache_for_channel', ErrataCache_queries.xml) instead.
> >> 
> >> Maybe someone of you who knows about this code could have a look at
> >> the issue, I might be missing something ..
> > 
> > Replying to add more background. I was not aware of this specific bug
> > myself either. I'd be interested to see if we can reproduce on Sat 5.4
> > code as well as current Spacewalk code.
> > 
> > This area of the code seems to get updates and re-factor once every
> > couple of years, due to subtle bugs in logic, along with
> > scalability/performance feedback.
> > 
> > The particular line you attached to propose changing was last changed in
> > Feb 2009 as part of a larger re-factor:
> > http://git.fedorahosted.org/git/?p=spacewalk.git;a=commitdiff;h=3707af41
> > 49d84a1ddb1ce693c78f7b791a044cd1
> > 
> > If memory serves me correct, we were in the middle of a Satellite
> > release cycle, so changing the stored procedure was not an ideal
> > solution for Satellite. We fixed Java code - which performed well.
> > 
> >  I'm not sure if I'd want the fix, as proposed, in 1.7, without us
> > 
> > spending time to confirm it is the best solution for logic and
> > performance. I don't think we will have time to dedicate to assist until
> > post 1.7. If one the guys finds we have time to dig in before 1.7 goes
> > out in the next week (hopefully) - we will.
> > 
> > Cliff
> 
> Sorry, I now found out I was experiencing this bug and was missing the
> patch:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=707658
> 
> So the cloned patches now show up correctly on the UI, but anyways I think
> there is some code in here that does strange things. E.g. when I analyze the
> file PublishErrataAction.java:
> 
> The newly cloned patch is published by calling in line 80:
> 
> ErrataManager.publishErrataToChannelAsync(...)
> 
> This also triggers insertion into the cache table ("rhnServerNeededCache").
> So why do we need to call later on (and without the actual errata ids):
> 
> ErrataCacheManager.insertCacheForChannelPackagesAsync(chanList, pidList);
> (line 100)
> 
> This seems to insert another row into rhnServerNeededCache where the eid is
> null! If this is wanted behaviour, then what is it for?

Right Johannes,

good catch.
this definitelly isn't wanted behavior.
If the added packages to a channals are associated with any erratum (what is 
our case), we definitelly want to consider also the erratum, when regenerating 
errata cache.

Will you find some time to verify the behavior, when removing the errata cache 
update on line 100?
If not, please, file a BZ for the issue.

> 
> Thanks,
> Johannes

Thank you,
-- 
Tomas Lestach
RHN Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] "All relevant errata" as "All Errata"

2012-03-05 Thread Duncan Mac-Vicar P.

On 03/02/2012 04:06 PM, Tomas Lestach wrote:

On Friday 02 of March 2012 15:41:49 Duncan Mac-Vicar P. wrote:

Hi Astronauts,

The errata hierarchy in the Errata tab is:

Errata
Relevant
  All Errata - Bugfix Errata - Enhancement Errata - Security Errata
All
  All Errata - Bugfix Errata - Enhancement Errata - Security Errata

When the user is in Relevant (Errata Relevant to Your System) having a
subtab "All Errata" is confusing, because it really means "Relevant
Errata of all types", and not "All Errata" like the parent menu.

I suggest to change it to "All Types" un both Errata->All and
Errata->Relevant.

Comments?

I agree.

Attached patch for english and spanish. In spanish I did not need to 
mention "all TYPES" as I can use the gender difference to make it very 
obvious if I am talking of erratas types or erratas.


German, is still being discussed here in the office...

Duncan

>From f6fcded8d4c718c1089d1aa31ee75196fd7a59db Mon Sep 17 00:00:00 2001
From: Duncan Mac-Vicar P 
Date: Mon, 5 Mar 2012 13:27:53 +0100
Subject: [PATCH 1/2] Rename "All Errata" to "All Types" as we are refering to
 Erratas of all types (Bugfixes, Security) and it can be
 confused with the "All Errata" menu which refers to
 Relevant/All.

See:
https://www.redhat.com/archives/spacewalk-devel/2012-March/thread.html#2
---
 .../frontend/strings/jsp/StringResource_en_US.xml  |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
index 2cc91d6..becd5e4 100644
--- a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
+++ b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
@@ -3500,7 +3500,7 @@ button below, and will be unable to log back in.
 
   
   
-All Errata
+All Types
 
   /rhn/errata/AllErrata
 
-- 
1.7.7

>From 27dae2a916f5aed5a59721492c38e625492db6c7 Mon Sep 17 00:00:00 2001
From: Duncan Mac-Vicar P 
Date: Mon, 5 Mar 2012 13:32:49 +0100
Subject: [PATCH 2/2] Rename "Toda la errata" to "Todos" as we are refering to
 Erratas of all types (Bugfixes, Security) and it can be
 confused with the "All Errata" menu which refers to
 Relevant/All.

See:
https://www.redhat.com/archives/spacewalk-devel/2012-March/thread.html#2

(Spanish version)
---
 .../rhn/frontend/strings/jsp/StringResource_es.xml |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_es.xml b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_es.xml
index 813db7f..767f5b9 100644
--- a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_es.xml
+++ b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_es.xml
@@ -3491,7 +3491,7 @@ del rol de Administrador de la organización.
 
   /rhn/errata/AllErrata
 
-  Toda la errata
+  Todos
 
 
   
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] "All relevant errata" as "All Errata"

2012-03-05 Thread Tomas Lestach
On Friday 02 of March 2012 18:13:29 Duncan Mac-Vicar P. wrote:
> On 03/02/2012 05:55 PM, Duncan Mac-Vicar P. wrote:
> > On 03/02/2012 04:06 PM, Tomas Lestach wrote:
> >> There's one more thing confusing for me, when we're in the errata
> >> area.
> >> I mean following two pages
> >> 
> >> https:///rhn/errata/Overview.do
> >> https:///rhn/errata/RelevantErrata.do
> >> 
> >> display the same errata content.
> >> We might want to drop one of them.
> > 
> > Ok, I have a patch that gets rid of errata/Overview.do. I need to
> > rebase for master.
> 
> attached.

Thank you!
Committed as: 86be01047a59b3f0120e30ce1af8fbaebc9b75cf

> 
> Is the removal of strings with this file as context needed?

That would be really nice, even if we usually do not remove the unused 
strings. :(

> 
> Duncan

Regards,
-- 
Tomas Lestach
RHN Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel