Ilya Kasnacheev wrote > I have noticed that both spec and DEVNOTES are made to use tabs alongside > with spaces. I thought we are avoiding tabs? Please confirm that usage of > tabs is desired in this case.
My fault - got used to editing such files in default vim. Fixed and updated vim settings to indent with spaces. Ilya Kasnacheev wrote > Maybe it's my CentOS but initially build will fail with the following > message, which I had to fix in spec > + cd 'apache-ignite-*' > /var/tmp/rpm-tmp.KwOoyl: line 34: cd: apache-ignite-*: No such file or > directory Strange error - shell expansion is not working. Anyway, replaced with more universal variant. Ilya Kasnacheev wrote > We absolutely should not inline configuration files (such as > default-config.xml) into build scripts. We should just copy over > default-config.xml from config/ to /etc, with dependencies if needed. Extracted corresponding sections into separate files. Ilya Kasnacheev wrote > I'm not sure PDS should be ON by default. Better provide it in examples > but disable by default. AFAIK, PDS is actively pushing forward and has to be enabled by default so that Ignite users start working with PDS from the very beginning. Ilya Kasnacheev wrote > I cannot comment on firewall rules validity, but firewall and service > configurations should be stored in sources, as files, supplied with > release (somewhere in packages/) and installed from there Unfortunately, I did not find any other way to open ports and multicast, except applying direct iptables rules though firewall-cmd -- so there can be no separate service firewall rules file (as can be found here: /usr/lib/firewalld/services). Applied rules from package go straight to /etc/firewalld/direct.xml. If we agree not to put Multicast-enabled by default configuration file and will stick to IP Discovery, I'll try to revise firewall configuration and create separate service firewall rules file (but, I guess, much later). Ilya Kasnacheev wrote > * sqlline.sh fails to connect initially, needs to be specified > jdbc:ignite:thin://localhost, then it works. I think that sqlline should > become available as /usr/bin/apache-ignite-sqlline for example, to be in > $PATH. And figure out how to connect by itself. > * ignitevisorcmd.sh becomes confused. first it scans > /usr/share/apache-ignite for configs, then it fails to write to > /usr/share/apache-ignite/work. We should consider how it can be fixed (a > symlink from /usr/share/apache-ignite/config to /etc/apache-ignite, > perhaps, and work dir in /tmp?), and made available as > /usr/bin/apache-ignite-visorcmd. The problem exists mainly because of sqlline.sh|ignitevisorcmd.sh unaware-of-package design and I see the following 3-step way to overcome this limitation. At first we hide this executables file in examples (As Iliya proposed). Secondly, I can design a patch, which can be applied during package build, so that those executables work normally form the package installation. And lastly, redesign them to be universal and compatible with all delivery methods. Ilya Kasnacheev wrote > Not everything should go into /usr/share. classpath libs belong to > /usr/lib. Moved libs to /usr/lib/apache-ignite and mapped to /usr/share/apache-ignite/libs (for backward compatibility). I guess later we have to think out of a solution to load libs directly from /usr/lib (along with configurations / logs / work / etc.) Ilya Kasnacheev wrote > We are building from binary package and not from sources. If it is used by > internal RPM building this is tolerable, but any distro or repository will > want to build from source. It's not an ordinary task to create "100% honest" package, so I guess we definitely won't make to 2.4 release. So I purpose include in the nearest release our package built from binary archive and do not supply sources package (as it is currently done in apache.org/dist for cassandra / hadoop or alike). And later design package build procedure, which includes getting sources from sources distribution, or even from Apache Ignite Git repository tag. Ilya Kasnacheev wrote > spec contains version (2.4.0) - will be needed to be changed for every > release. I'm not sure if it's possible to extract. Release procedure on TC is already has some task for project version update, so I guess it is not so difficult to update it accordingly (as it is currently done for C++ / .Net internal versions). Ilya Kasnacheev wrote > Package description may be improved. service description lacks '-' in "In > Memory". Fixed. So the only question to discuss (according Iliya's review) is whether to supply package with default-config.xml that has Multicast Discovery turned on or not? Also I'll appreciate any other comments concerning current package design. -- Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
