[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #18 from Thomas Andrejak  ---
Indeed, really strange I don't see this. Thanks !

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226

Fedora Update System  changed:

   What|Removed |Added

 Status|ON_QA   |CLOSED
 Resolution|--- |ERRATA
Last Closed||2017-03-10 06:20:06



--- Comment #17 from Fedora Update System  ---
prelude-correlator-3.1.0-1.el7 has been pushed to the Fedora EPEL 7 stable
repository. If problems still persist, please make note of it in this bug
report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #16 from Fedora Update System  ---
prelude-correlator-3.1.0-1.fc25 has been pushed to the Fedora 25 stable
repository. If problems still persist, please make note of it in this bug
report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #15 from Fedora Update System  ---
prelude-correlator-3.1.0-1.fc24 has been pushed to the Fedora 24 stable
repository. If problems still persist, please make note of it in this bug
report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226

Tuomo Soini  changed:

   What|Removed |Added

 CC||t...@foobar.fi



--- Comment #14 from Tuomo Soini  ---
Epel7 testing package is broken.

package: python3-prelude-correlator-3.1.0-1.el7.noarch
  unresolved deps: 
 python3-prelude >= 0:3.1.0
 python3-netaddr

python3-prelude-correlator should be be
python%{python3_pkgversion}-prelude-correlator.

And those dependencies should use %{python3_pkgversion} macro.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226

Fedora Update System  changed:

   What|Removed |Added

 Status|MODIFIED|ON_QA



--- Comment #13 from Fedora Update System  ---
prelude-correlator-3.1.0-1.el7 has been pushed to the Fedora EPEL 7 testing
repository. If problems still persist, please make note of it in this bug
report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-fafb6a6624

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #12 from Fedora Update System  ---
prelude-correlator-3.1.0-1.el7 has been submitted as an update to Fedora EPEL
7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-fafb6a6624

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226

Fedora Update System  changed:

   What|Removed |Added

 Status|ON_QA   |MODIFIED



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #11 from Fedora Update System  ---
prelude-correlator-3.1.0-1.fc25 has been pushed to the Fedora 25 testing
repository. If problems still persist, please make note of it in this bug
report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-73969f8acd

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226

Fedora Update System  changed:

   What|Removed |Added

 Status|MODIFIED|ON_QA



--- Comment #10 from Fedora Update System  ---
prelude-correlator-3.1.0-1.fc24 has been pushed to the Fedora 24 testing
repository. If problems still persist, please make note of it in this bug
report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-79dc3a256d

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #9 from Fedora Update System  ---
prelude-correlator-3.1.0-1.fc25 has been submitted as an update to Fedora 25.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-73969f8acd

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226

Fedora Update System  changed:

   What|Removed |Added

 Status|POST|MODIFIED



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #8 from Fedora Update System  ---
prelude-correlator-3.1.0-1.fc24 has been submitted as an update to Fedora 24.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-79dc3a256d

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226

Zbigniew Jędrzejewski-Szmek  changed:

   What|Removed |Added

 Status|ASSIGNED|POST
  Flags|fedora-review?  |fedora-review+



--- Comment #7 from Zbigniew Jędrzejewski-Szmek  ---
So now there's a circular dependency, between the main package and the python3
subpackage. That's a bit unusual, but OTOH, it doesn't cause any real issues.

+ package name is OK
+ license is acceptable for Fedora (GPLv2+)
+ license is specified correctly
+ provides and requires are OK
+ scriptlets are OK
+ builds and installs OK

Package is APPROVED.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #6 from Thomas Andrejak  ---
Hello,

Here are the new files :
SPEC : https://fedorapeople.org/~totol/prelude-correlator.spec
SRPM : https://fedorapeople.org/~totol/prelude-correlator-3.1.0-1.fc26.src.rpm

(In reply to Zbigniew Jędrzejewski-Szmek from comment #5)
> > Prelude-Correlator seems to have this functionnality but it dit it in the 
> > wrong way. I prefere not to activate it and wait for upstream.
> 
> With systemd's User= setting, running as non-root requires no special
> support from the application, often. This gives an excellent ratio of
> security gain vs. inconvenience.
> 
> That said, if you prefer to wait for upstream, that makes sense too.
> (Although they are more likely to implement it "portably", i.e. from
> scratch, and mess something up ;))

I understand but for example, if the root user change things in
/etc/prelude-correlator , prelude-correlator may stop working because it can
not access / remove / modify files. For now, I prefer let it in this way if it
is possible.

> 
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
> > s/SystemD/systemd/
> Still there.
> 

I don't see it in your comment, sorry. Done

> >> Hm, why do the python subpackage require prelude-correlator.rpm?
> >> prelude-correlator includes a service file which will not work without
> >> python3-p-c, so it seems prelude-correlator.rpm should require
> >> python3-prelude-correlator. I'm pretty sure that 'dnf install
> >> prelude-correlator' should yield a working setup.
> > You are right, Done
> 
> I don't think that's good as is. I think the main package (%name.rpm) should
> have
> Requires: python3-%{name}
> 
> (Otherwise, if you do dnf install %{name}, systemctl start %{name}, this
> will crash on failed import).

I write it too quickly. It is now OK.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #5 from Zbigniew Jędrzejewski-Szmek  ---
> Prelude-Correlator seems to have this functionnality but it dit it in the 
> wrong way. I prefere not to activate it and wait for upstream.

With systemd's User= setting, running as non-root requires no special support
from the application, often. This gives an excellent ratio of security gain vs.
inconvenience.

That said, if you prefer to wait for upstream, that makes sense too. (Although
they are more likely to implement it "portably", i.e. from scratch, and mess
something up ;))

(In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
> s/SystemD/systemd/
Still there.

>> Hm, why do the python subpackage require prelude-correlator.rpm?
>> prelude-correlator includes a service file which will not work without
>> python3-p-c, so it seems prelude-correlator.rpm should require
>> python3-prelude-correlator. I'm pretty sure that 'dnf install
>> prelude-correlator' should yield a working setup.
> You are right, Done

I don't think that's good as is. I think the main package (%name.rpm) should
have
Requires: python3-%{name}

(Otherwise, if you do dnf install %{name}, systemctl start %{name}, this will
crash on failed import).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #4 from Thomas Andrejak  ---
Hello,

Here are the new files :
SPEC : https://fedorapeople.org/~totol/prelude-correlator.spec
SRPM : https://fedorapeople.org/~totol/prelude-correlator-3.1.0-1.fc26.src.rpm

(In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
> s/SystemD/systemd/
> 
> + package name is OK
> + license is acceptable (GPLv2)
> - license is specified correctly:
>   not, it's GPLv2+, see the headers in the files

Indeed, Done

> 
> Why /etc/prelude-correlator/rules? That python code does not look like
> something that is configurable, wouldn't it be better to put it somewhere in
> /usr/?

This is python files but this is really configuration files. This is where the
user describes it's correlation rules. The user can add rules, delete rules,
modify rules.

> 
> Does the binary fork anything? If there's just one process, you can do away
> with the pid file, which would obviate the need for a directory in /run and
> simplify things.

Yes, Done

> 
> Could this daemon run as a normal user? What kind of privileges does is
> require?

Prelude-Correlator seems to have this functionnality but it dit it in the wrong
way. I prefere not to activate it and wait for upstream.

> 
> Hm, why do the python subpackage require prelude-correlator.rpm?
> prelude-correlator includes a service file which will not work without
> python3-p-c, so it seems prelude-correlator.rpm should require
> python3-prelude-correlator. I'm pretty sure that 'dnf install
> prelude-correlator' should yield a working setup.

You are right, Done

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #3 from Zbigniew Jędrzejewski-Szmek  ---
s/SystemD/systemd/

+ package name is OK
+ license is acceptable (GPLv2)
- license is specified correctly:
  not, it's GPLv2+, see the headers in the files

Why /etc/prelude-correlator/rules? That python code does not look like
something that is configurable, wouldn't it be better to put it somewhere in
/usr/?

Does the binary fork anything? If there's just one process, you can do away
with the pid file, which would obviate the need for a directory in /run and
simplify things.

Could this daemon run as a normal user? What kind of privileges does is
require?

Hm, why do the python subpackage require prelude-correlator.rpm?
prelude-correlator includes a service file which will not work without
python3-p-c, so it seems prelude-correlator.rpm should require
python3-prelude-correlator. I'm pretty sure that 'dnf install
prelude-correlator' should yield a working setup.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226



--- Comment #2 from Thomas Andrejak  ---
Hello,

Here are the new files :
SPEC : https://fedorapeople.org/~totol/prelude-correlator.spec
SRPM : https://fedorapeople.org/~totol/prelude-correlator-3.1.0-1.fc26.src.rpm

(In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
> No python 3 support? Please note that python3 support MUST be enabled if
> possible.

=> I had a patch to have it :)

> 
> "install init script" → not to be picky, but it's neither init nor a script
> ;)

Done

> 
> In one place you use install -D, in others the directories are created by
> hand.

Done

> 
> It looks all good, only cosmetic issues, apart from the one big question —
> python3.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1419226] Review Request: prelude-correlator

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1419226

Zbigniew Jędrzejewski-Szmek  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
 CC||zbys...@in.waw.pl
   Assignee|nob...@fedoraproject.org|zbys...@in.waw.pl
  Flags||fedora-review?



--- Comment #1 from Zbigniew Jędrzejewski-Szmek  ---
No python 3 support? Please note that python3 support MUST be enabled if
possible.

"install init script" → not to be picky, but it's neither init nor a script ;)

In one place you use install -D, in others the directories are created by hand.

It looks all good, only cosmetic issues, apart from the one big question —
python3.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org