Re: [DISCUSSION] Code style. Variable abbrevations

2021-06-16 Thread Ivan Pavlukhin
Hi Nikolay,

Thanks, it is rather interesting.

Do we all agree that using different conventions for different code
packages does not break "consistency"? Or did I get something wrong?

2021-06-17 7:12 GMT+03:00, Николай Ижиков :
> Hello, Ivan.
>
> We can create checkstyle rule to enforce usage of abbreviations.
> Internal/public code differs by package.
>
> PoC of rule [1]
>
> [1] https://github.com/apache/ignite/pull/9153
>
>> 17 июня 2021 г., в 07:01, Ivan Pavlukhin 
>> написал(а):
>>
>> Nikita,
>>
>> Do you have a plan in your mind how to make Ignite codebase consistent?
>>
>> AFAIR, we had it intentionally inconsistent for a long time at least
>> for one sake: for internal code we used special conventions (e.g.
>> abbreviations, shorten getters) and common Java conventions for public
>> API and examples (e.g. no abbreviations and usual getters).
>>
>> 2021-06-16 23:37 GMT+03:00, Nikita Ivanov :
>>> Consistency is what makes it easier to contribute to the project and
>>> attract new members. Consistency implies strong, well defined and
>>> universally enforced rules. Just because we have some individuals who
>>> are lazy or inexperienced - it does not mean that the entire project
>>> should relax the basic-level engineering discipline.
>>>
>>> On a personal node - nothing screams "immaturity" louder that a code
>>> that uses inconsistent naming, commenting, code style & organization,
>>> etc.
>>> --
>>> Nikita Ivanov
>>>
 On Wed, Jun 16, 2021 at 5:56 AM Andrey Gura  wrote:

 Hi,

 I understand that this rule seems too strict or may be weird. But
 removing the rule could lead to review comments like "use future
 instead of fut". So my proposal is to change rule from "required" to
 "recommended".

 On Wed, Jun 16, 2021 at 2:49 PM Valentin Kulichenko
  wrote:
>
> Konstantin, thanks for chiming in.
>
> That's exactly my concern. Overformalization is generally not a good
> thing.
> Someone used "mess" to abbreviate "message"? That is surely not a good
> coding style, but that's what code reviews are for. I believe that our
> committers are more than capable of identifying such issues.
>
> At the same time, with the current rules, we are *forced* to use
> abbreviations like "locAddrGrpMgr", whether we like it or not. All it
> does
> is makes it harder to contribute to Ignite, without providing any
> clear
> value.
>
> -Val
>
> On Thu, Jun 10, 2021 at 9:46 AM Konstantin Orlov 
> wrote:
>
>> +1 for making this optional
>>
>> Common abbreviation rules is good for sure, but sometimes I getting
>> sick
>> of all those locAddrGrpMgr.
>>
>>
>> --
>> Regards,
>> Konstantin Orlov
>>
>>
>>
>>
>>> On 7 Jun 2021, at 14:31, Nikolay Izhikov 
>>> wrote:
>>>
>>> Hello, Anton, Alexei.
>>>
>>> Thanks for the feedback.
>>>
>>> Personally, I’m pretty happy current abbreviation rules too.
>>> Let see what we can do to make our codebase even more consistent.
>>>
 7 июня 2021 г., в 13:23, Alexei Scherbakov <
>> alexey.scherbak...@gmail.com> написал(а):

 -1
 Common abbrevs add quality to the code.

 пн, 7 июн. 2021 г. в 12:38, Anton Vinogradov :

> -1 here.
> We can fix the code and set up the rule.
>
> This will help to prevent having a weird abbreviation like "mess"
> (from
> "message") or "ign" (from "Ignite").
> Also, the abbreviations list (hardcoded at IDEA plugin) allows to
> have
>> same
> names across the whole code, this should simplify the reading.
>
> On Sat, Jun 5, 2021 at 10:49 PM Valentin Kulichenko <
> valentin.kuliche...@gmail.com> wrote:
>
>> I also support removing this requirement. It’s not the first
>> time
>> someone
>> brings this up, and so far we haven’t been able to fix it. Not
>> worth
>> it
> in
>> my view.
>>
>> -Val
>>
>> On Sat, Jun 5, 2021 at 11:54 AM Nikolay Izhikov
>> 
>> wrote:
>>
>>> Hello, guys.
>>>
>>> Thanks for the feedback.
>>>
>>> Dmitry,
>>>
 Manual rule enforcement saves a couple of symbols in code
>>>
>>> I’m talking about automatic check.
>>> We can implement it easily.
>>> So, if we decide to keep this rule all we need is to fix
>>> current
>>> violations (several thousand!).
>>> After it, checkstyle will automatically enforce this rule.
>>> As you may know, currently, any PR checked according to our
>> checkstyle
>>> rules.
>>> Please, take a look at little green check sign after PR name.
>>>
>>>
 5 июня 2021 г., в 

Re: [DISCUSSION] Code style. Variable abbrevations

2021-06-16 Thread Николай Ижиков
Hello, Ivan.

We can create checkstyle rule to enforce usage of abbreviations.
Internal/public code differs by package.

PoC of rule [1]

[1] https://github.com/apache/ignite/pull/9153

> 17 июня 2021 г., в 07:01, Ivan Pavlukhin  написал(а):
> 
> Nikita,
> 
> Do you have a plan in your mind how to make Ignite codebase consistent?
> 
> AFAIR, we had it intentionally inconsistent for a long time at least
> for one sake: for internal code we used special conventions (e.g.
> abbreviations, shorten getters) and common Java conventions for public
> API and examples (e.g. no abbreviations and usual getters).
> 
> 2021-06-16 23:37 GMT+03:00, Nikita Ivanov :
>> Consistency is what makes it easier to contribute to the project and
>> attract new members. Consistency implies strong, well defined and
>> universally enforced rules. Just because we have some individuals who
>> are lazy or inexperienced - it does not mean that the entire project
>> should relax the basic-level engineering discipline.
>> 
>> On a personal node - nothing screams "immaturity" louder that a code
>> that uses inconsistent naming, commenting, code style & organization,
>> etc.
>> --
>> Nikita Ivanov
>> 
>>> On Wed, Jun 16, 2021 at 5:56 AM Andrey Gura  wrote:
>>> 
>>> Hi,
>>> 
>>> I understand that this rule seems too strict or may be weird. But
>>> removing the rule could lead to review comments like "use future
>>> instead of fut". So my proposal is to change rule from "required" to
>>> "recommended".
>>> 
>>> On Wed, Jun 16, 2021 at 2:49 PM Valentin Kulichenko
>>>  wrote:
 
 Konstantin, thanks for chiming in.
 
 That's exactly my concern. Overformalization is generally not a good
 thing.
 Someone used "mess" to abbreviate "message"? That is surely not a good
 coding style, but that's what code reviews are for. I believe that our
 committers are more than capable of identifying such issues.
 
 At the same time, with the current rules, we are *forced* to use
 abbreviations like "locAddrGrpMgr", whether we like it or not. All it
 does
 is makes it harder to contribute to Ignite, without providing any clear
 value.
 
 -Val
 
 On Thu, Jun 10, 2021 at 9:46 AM Konstantin Orlov 
 wrote:
 
> +1 for making this optional
> 
> Common abbreviation rules is good for sure, but sometimes I getting
> sick
> of all those locAddrGrpMgr.
> 
> 
> --
> Regards,
> Konstantin Orlov
> 
> 
> 
> 
>> On 7 Jun 2021, at 14:31, Nikolay Izhikov 
>> wrote:
>> 
>> Hello, Anton, Alexei.
>> 
>> Thanks for the feedback.
>> 
>> Personally, I’m pretty happy current abbreviation rules too.
>> Let see what we can do to make our codebase even more consistent.
>> 
>>> 7 июня 2021 г., в 13:23, Alexei Scherbakov <
> alexey.scherbak...@gmail.com> написал(а):
>>> 
>>> -1
>>> Common abbrevs add quality to the code.
>>> 
>>> пн, 7 июн. 2021 г. в 12:38, Anton Vinogradov :
>>> 
 -1 here.
 We can fix the code and set up the rule.
 
 This will help to prevent having a weird abbreviation like "mess"
 (from
 "message") or "ign" (from "Ignite").
 Also, the abbreviations list (hardcoded at IDEA plugin) allows to
 have
> same
 names across the whole code, this should simplify the reading.
 
 On Sat, Jun 5, 2021 at 10:49 PM Valentin Kulichenko <
 valentin.kuliche...@gmail.com> wrote:
 
> I also support removing this requirement. It’s not the first
> time
> someone
> brings this up, and so far we haven’t been able to fix it. Not
> worth
> it
 in
> my view.
> 
> -Val
> 
> On Sat, Jun 5, 2021 at 11:54 AM Nikolay Izhikov
> 
> wrote:
> 
>> Hello, guys.
>> 
>> Thanks for the feedback.
>> 
>> Dmitry,
>> 
>>> Manual rule enforcement saves a couple of symbols in code
>> 
>> I’m talking about automatic check.
>> We can implement it easily.
>> So, if we decide to keep this rule all we need is to fix
>> current
>> violations (several thousand!).
>> After it, checkstyle will automatically enforce this rule.
>> As you may know, currently, any PR checked according to our
> checkstyle
>> rules.
>> Please, take a look at little green check sign after PR name.
>> 
>> 
>>> 5 июня 2021 г., в 00:57, Dmitry Pavlov 
> написал(а):
>>> 
>>> +1 for removal. Cnt and count both seem to be fine.
>>> 
>>> Manual rule enforcement saves a couple of symbols in code, but
 requires
>> some time from both contributor and reviewer.
>>> 
>>> Sincerely,
>>> Dmitriy Pavlov

Re: [DISCUSSION] Code style. Variable abbrevations

2021-06-16 Thread Ivan Pavlukhin
Nikita,

Do you have a plan in your mind how to make Ignite codebase consistent?

AFAIR, we had it intentionally inconsistent for a long time at least
for one sake: for internal code we used special conventions (e.g.
abbreviations, shorten getters) and common Java conventions for public
API and examples (e.g. no abbreviations and usual getters).

2021-06-16 23:37 GMT+03:00, Nikita Ivanov :
> Consistency is what makes it easier to contribute to the project and
> attract new members. Consistency implies strong, well defined and
> universally enforced rules. Just because we have some individuals who
> are lazy or inexperienced - it does not mean that the entire project
> should relax the basic-level engineering discipline.
>
> On a personal node - nothing screams "immaturity" louder that a code
> that uses inconsistent naming, commenting, code style & organization,
> etc.
> --
> Nikita Ivanov
>
> On Wed, Jun 16, 2021 at 5:56 AM Andrey Gura  wrote:
>>
>> Hi,
>>
>> I understand that this rule seems too strict or may be weird. But
>> removing the rule could lead to review comments like "use future
>> instead of fut". So my proposal is to change rule from "required" to
>> "recommended".
>>
>> On Wed, Jun 16, 2021 at 2:49 PM Valentin Kulichenko
>>  wrote:
>> >
>> > Konstantin, thanks for chiming in.
>> >
>> > That's exactly my concern. Overformalization is generally not a good
>> > thing.
>> > Someone used "mess" to abbreviate "message"? That is surely not a good
>> > coding style, but that's what code reviews are for. I believe that our
>> > committers are more than capable of identifying such issues.
>> >
>> > At the same time, with the current rules, we are *forced* to use
>> > abbreviations like "locAddrGrpMgr", whether we like it or not. All it
>> > does
>> > is makes it harder to contribute to Ignite, without providing any clear
>> > value.
>> >
>> > -Val
>> >
>> > On Thu, Jun 10, 2021 at 9:46 AM Konstantin Orlov 
>> > wrote:
>> >
>> > > +1 for making this optional
>> > >
>> > > Common abbreviation rules is good for sure, but sometimes I getting
>> > > sick
>> > > of all those locAddrGrpMgr.
>> > >
>> > >
>> > > --
>> > > Regards,
>> > > Konstantin Orlov
>> > >
>> > >
>> > >
>> > >
>> > > > On 7 Jun 2021, at 14:31, Nikolay Izhikov 
>> > > > wrote:
>> > > >
>> > > > Hello, Anton, Alexei.
>> > > >
>> > > > Thanks for the feedback.
>> > > >
>> > > > Personally, I’m pretty happy current abbreviation rules too.
>> > > > Let see what we can do to make our codebase even more consistent.
>> > > >
>> > > >> 7 июня 2021 г., в 13:23, Alexei Scherbakov <
>> > > alexey.scherbak...@gmail.com> написал(а):
>> > > >>
>> > > >> -1
>> > > >> Common abbrevs add quality to the code.
>> > > >>
>> > > >> пн, 7 июн. 2021 г. в 12:38, Anton Vinogradov :
>> > > >>
>> > > >>> -1 here.
>> > > >>> We can fix the code and set up the rule.
>> > > >>>
>> > > >>> This will help to prevent having a weird abbreviation like "mess"
>> > > >>> (from
>> > > >>> "message") or "ign" (from "Ignite").
>> > > >>> Also, the abbreviations list (hardcoded at IDEA plugin) allows to
>> > > >>> have
>> > > same
>> > > >>> names across the whole code, this should simplify the reading.
>> > > >>>
>> > > >>> On Sat, Jun 5, 2021 at 10:49 PM Valentin Kulichenko <
>> > > >>> valentin.kuliche...@gmail.com> wrote:
>> > > >>>
>> > >  I also support removing this requirement. It’s not the first
>> > >  time
>> > > someone
>> > >  brings this up, and so far we haven’t been able to fix it. Not
>> > >  worth
>> > > it
>> > > >>> in
>> > >  my view.
>> > > 
>> > >  -Val
>> > > 
>> > >  On Sat, Jun 5, 2021 at 11:54 AM Nikolay Izhikov
>> > >  
>> > >  wrote:
>> > > 
>> > > > Hello, guys.
>> > > >
>> > > > Thanks for the feedback.
>> > > >
>> > > > Dmitry,
>> > > >
>> > > >> Manual rule enforcement saves a couple of symbols in code
>> > > >
>> > > > I’m talking about automatic check.
>> > > > We can implement it easily.
>> > > > So, if we decide to keep this rule all we need is to fix
>> > > > current
>> > > > violations (several thousand!).
>> > > > After it, checkstyle will automatically enforce this rule.
>> > > > As you may know, currently, any PR checked according to our
>> > > checkstyle
>> > > > rules.
>> > > > Please, take a look at little green check sign after PR name.
>> > > >
>> > > >
>> > > >> 5 июня 2021 г., в 00:57, Dmitry Pavlov 
>> > >  написал(а):
>> > > >>
>> > > >> +1 for removal. Cnt and count both seem to be fine.
>> > > >>
>> > > >> Manual rule enforcement saves a couple of symbols in code, but
>> > > >>> requires
>> > > > some time from both contributor and reviewer.
>> > > >>
>> > > >> Sincerely,
>> > > >> Dmitriy Pavlov
>> > > >>
>> > > >> On 2021/06/04 19:18:37, Pavel Tupitsyn 
>> > > wrote:
>> > > >>> In my opinion, we should remove this rule.
>> > > >>> Looks like a 

Re: [DISCUSSION] Code style. Variable abbrevations

2021-06-16 Thread Nikita Ivanov
Consistency is what makes it easier to contribute to the project and
attract new members. Consistency implies strong, well defined and
universally enforced rules. Just because we have some individuals who
are lazy or inexperienced - it does not mean that the entire project
should relax the basic-level engineering discipline.

On a personal node - nothing screams "immaturity" louder that a code
that uses inconsistent naming, commenting, code style & organization,
etc.
--
Nikita Ivanov

On Wed, Jun 16, 2021 at 5:56 AM Andrey Gura  wrote:
>
> Hi,
>
> I understand that this rule seems too strict or may be weird. But
> removing the rule could lead to review comments like "use future
> instead of fut". So my proposal is to change rule from "required" to
> "recommended".
>
> On Wed, Jun 16, 2021 at 2:49 PM Valentin Kulichenko
>  wrote:
> >
> > Konstantin, thanks for chiming in.
> >
> > That's exactly my concern. Overformalization is generally not a good thing.
> > Someone used "mess" to abbreviate "message"? That is surely not a good
> > coding style, but that's what code reviews are for. I believe that our
> > committers are more than capable of identifying such issues.
> >
> > At the same time, with the current rules, we are *forced* to use
> > abbreviations like "locAddrGrpMgr", whether we like it or not. All it does
> > is makes it harder to contribute to Ignite, without providing any clear
> > value.
> >
> > -Val
> >
> > On Thu, Jun 10, 2021 at 9:46 AM Konstantin Orlov 
> > wrote:
> >
> > > +1 for making this optional
> > >
> > > Common abbreviation rules is good for sure, but sometimes I getting sick
> > > of all those locAddrGrpMgr.
> > >
> > >
> > > --
> > > Regards,
> > > Konstantin Orlov
> > >
> > >
> > >
> > >
> > > > On 7 Jun 2021, at 14:31, Nikolay Izhikov  wrote:
> > > >
> > > > Hello, Anton, Alexei.
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > Personally, I’m pretty happy current abbreviation rules too.
> > > > Let see what we can do to make our codebase even more consistent.
> > > >
> > > >> 7 июня 2021 г., в 13:23, Alexei Scherbakov <
> > > alexey.scherbak...@gmail.com> написал(а):
> > > >>
> > > >> -1
> > > >> Common abbrevs add quality to the code.
> > > >>
> > > >> пн, 7 июн. 2021 г. в 12:38, Anton Vinogradov :
> > > >>
> > > >>> -1 here.
> > > >>> We can fix the code and set up the rule.
> > > >>>
> > > >>> This will help to prevent having a weird abbreviation like "mess" 
> > > >>> (from
> > > >>> "message") or "ign" (from "Ignite").
> > > >>> Also, the abbreviations list (hardcoded at IDEA plugin) allows to have
> > > same
> > > >>> names across the whole code, this should simplify the reading.
> > > >>>
> > > >>> On Sat, Jun 5, 2021 at 10:49 PM Valentin Kulichenko <
> > > >>> valentin.kuliche...@gmail.com> wrote:
> > > >>>
> > >  I also support removing this requirement. It’s not the first time
> > > someone
> > >  brings this up, and so far we haven’t been able to fix it. Not worth
> > > it
> > > >>> in
> > >  my view.
> > > 
> > >  -Val
> > > 
> > >  On Sat, Jun 5, 2021 at 11:54 AM Nikolay Izhikov 
> > >  wrote:
> > > 
> > > > Hello, guys.
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > Dmitry,
> > > >
> > > >> Manual rule enforcement saves a couple of symbols in code
> > > >
> > > > I’m talking about automatic check.
> > > > We can implement it easily.
> > > > So, if we decide to keep this rule all we need is to fix current
> > > > violations (several thousand!).
> > > > After it, checkstyle will automatically enforce this rule.
> > > > As you may know, currently, any PR checked according to our
> > > checkstyle
> > > > rules.
> > > > Please, take a look at little green check sign after PR name.
> > > >
> > > >
> > > >> 5 июня 2021 г., в 00:57, Dmitry Pavlov 
> > >  написал(а):
> > > >>
> > > >> +1 for removal. Cnt and count both seem to be fine.
> > > >>
> > > >> Manual rule enforcement saves a couple of symbols in code, but
> > > >>> requires
> > > > some time from both contributor and reviewer.
> > > >>
> > > >> Sincerely,
> > > >> Dmitriy Pavlov
> > > >>
> > > >> On 2021/06/04 19:18:37, Pavel Tupitsyn 
> > > wrote:
> > > >>> In my opinion, we should remove this rule.
> > > >>> Looks like a waste of time. freq or frequency, cnt or count, it is
> > >  fine
> > > >>> either way.
> > > >>>
> > > >>> On Fri, Jun 4, 2021 at 7:39 PM Nikolay Izhikov <
> > > nizhi...@apache.org
> > > 
> > > > wrote:
> > > >>>
> > >  Hello, Igniters.
> > > 
> > >  Right now, we have the rule to use some predefined list of
> > >  abbrevation
> > > > for
> > >  variable names [1].
> > >  Some of the reviewers ask to follow this rule strictly.
> > > 
> > > > It is required to use abbreviated form for code consistency.
> > > 
> > > 

Re: [DISCUSSION] Javadoc style requirements and checks in Ignite-3.

2021-06-16 Thread Nikita Ivanov
Hi,
In my strong opinion Javadoc is a code. Any errors in Javadoc are
equal to the bug in the code and must be treated as such. Proper and
extensive Javadoc for all public and many private APIs is absolutely
essential for this project, its growth and maturity.

I'm surprised this community still needs to debate fundamental
engineering issues like that...
--
Nikita Ivanov

On Wed, Jun 16, 2021 at 4:47 AM Andrey Gura  wrote:
>
> Hi,
>
> I think that scope should be limited by public API  for beginning.
> Also I don't sure that we should support specific tags like @apiNote,
> @implSpec, @implNote.
>
> Let's move using the following plan:
>
> 1. Create an empty (effectively) checkstyle config for javadoc
> checking and commit it to the repository. After this step it will be
> possible to configure TC in order to use this configuration.
> 2. Configure TC.
> 3. Commit a non-empty checkstyle config, but all modules should be
> excluded from checking on this step.
> 4. For each module create a JIRA issue. Module maintainers fix
> corresponding tickets and remove module exclusion from checking.
> 5. Add information about javadoc validation targets to DEVNOTES.md
> file (could be done on step 3)
>
> Any objections?
>
> On Tue, Apr 20, 2021 at 11:40 AM Andrey Mashenkov
>  wrote:
> >
> > I've fixed the PR.
> >
> > Now,
> > 1. Javadoc style is only checked if it is present for the element. All
> > declared javadoc tags MUST have a description.
> > So, one should either write correct javadoc or don't write at all.
> > 2. Additional checks performed for non-internal and non-test classes. All
> > public and protected elements must have non-emtpy javadoc.
> >
> > So, checkstyle detects 500+ missed descriptions (missed javadocs, tags
> > descriptions, tags themselves) in public scope
> > and 180+ bad-styles (missed brased, spaces, empty-lines) javadocs in whole
> > codebase.
> >
> > I'd suggest to force non-empty javadocs for all non-test classes including
> > internal (but their members) as well.
> >
> >
> >
> > On Mon, Apr 19, 2021 at 6:37 PM Andrey Mashenkov 
> > 
> > wrote:
> >
> > > Alexey,
> > >
> > > These are bad examples and unfortunately, most of the Ignite code doesn't
> > > look self-descriptive.
> > > (Do you feel the difference between @SerializableTransient and
> > > @TransientSerializable?)
> > >
> > > To understand the semantic of e.g. 'metricsHistSize' you have to analyze
> > > its usages.
> > > Getter and setter for this property look better, but it still not clear
> > > what happens with metric values above the limit?
> > >
> > > I have another example, one of the core components 
> > > GridDhtPartitionDemander
> > > has totally useless javadoc.
> > > It is obviously has nothing with thread pool + "local cache" phrase looks
> > > very confusing.
> > >
> > > /**
> > >  * Thread pool for requesting partitions from other nodes and populating 
> > > local cache.
> > >  */
> > > public class GridDhtPartitionDemander
> > >
> > > To understand the purpose of this internal component I have to analyze its
> > > code and usages.
> > > However, if one will face unexpected behavior, he/she may be confused if
> > > it is a lack of javadoc or wrong behavior.
> > >
> > > There is no way to restrict or avoid such issues with any checks.
> > > Agree, meaningful javadocs as self-descriptive code is more about culture
> > > and discipline.
> > >
> > > The absence of javadoc on internal API, unreasonably raises the entry
> > > threshold for new developers and makes the development of intra-component
> > > interaction harder.
> > > I admit a high-level description for public classes may be enough to
> > > resolve this without pushing developers to write empty/useless docs for
> > > every method/field.
> > >
> > >
> > > On Mon, Apr 19, 2021 at 5:21 PM Alexey Kukushkin <
> > > kukushkinale...@gmail.com> wrote:
> > >
> > >> I personally do not understand why we need mandatory javadoc even in
> > >> public
> > >> modules. I think javadoc is needed only when the code is not
> > >> self-descriptive. What value does a javadoc like this bring
> > >> <
> > >> https://github.com/apache/ignite/blob/2.10.0/modules/core/src/main/java/org/apache/ignite/configuration/IgniteConfiguration.java
> > >> >
> > >> to a developer:
> > >>
> > >> /** Default metrics history size (value is {@code 1}). */
> > >> public static final int DFLT_METRICS_HISTORY_SIZE = 1;
> > >>
> > >> /** Metrics history time. */
> > >> private int metricsHistSize = DFLT_METRICS_HISTORY_SIZE;
> > >>
> > >> BTW, I picked a random example and it already has a copy-paste mistake in
> > >> the javadoc, which is there for years. I think that indicates it has no
> > >> real value and developers just do it to comply with the rule.
> > >>
> > >> Some say mandatory javadoc is for the discipline but I think Apache 
> > >> Ignite
> > >> developers are mature enough to understand when additional documentation
> > >> is
> > >> really required.
> > >>
> > >>
> > >>
> > >> пн, 

Re: [VOTE] Release pyignite 0.5.0-rc1

2021-06-16 Thread Ivan Daschinsky
The vote will end at June, 17 15:00 UTC.

ср, 16 июн. 2021 г. в 17:33, Ivan Daschinsky :
>
> Dear Igniters!
>
> Release candidate binaries for subj are uploaded and ready for vote
> You can find them here:
> https://dist.apache.org/repos/dist/dev/ignite/pyignite/0.5.0-rc1
>
> If you follow the link above, you will find source package (*.tar.gz and 
> *.zip)
> and binary packages (wheels) for windows (amd64) and linux (x86_64)
> for pythons 36, 37, 38, 39. Also, there are sha512 and gpg signatures.
> Code signing keys can be found here -- 
> https://downloads.apache.org/ignite/KEYS
> Here you can find instructions how to verify packages
> https://www.apache.org/info/verification.html
>
> You can install binary package for specific version of python using pip
> For example do this on linux for python 3.8
> >> pip install pyignite-0.5.0-cp38-cp38-manylinux1_x86_64.whl
>
> You can build and install package from source using this command:
> >> pip install pyignite-0.5.0.tar.gz
> You can build wheel on your platform using this command:
> >> pip wheel --no-deps pyignite-0.5.0.tar.gz
>
> For building C module, you should have python headers and C compiler 
> installed.
> (i.e. for ubuntu sudo apt install build-essential python3-dev)
> In Mac OS X xcode-tools and python from homebrew are the best option.
>
> In order to check whether C module works, use following:
> >> from pyignite import _cutils
> >> print(_cutils.hashcode('test'))
> >> 3556498
>
> You can find documentation here:
> https://apache-ignite-binary-protocol-client.readthedocs.io/en/0.5.0.rc1
>
> You can find examples here (to check them, you should start ignite locally):
> https://apache-ignite-binary-protocol-client.readthedocs.io/en/0.5.0.rc1/examples.html
> Also, examples can be found in source archive in examples subfolder.
> docker-compose.yml is supplied in order to start ignite quickly. (Use
> `docker-compose up -d` to start 3 nodes cluster and `docker-compose
> down` to shut down it)
>
> Release notes:
> https://gitbox.apache.org/repos/asf?p=ignite-python-thin-client.git;a=blob;f=RELEASE_NOTES.txt;h=9d2ae81af2de22ce9e8c9d3b7ece14dd9e75ca0e;hb=61c83cb0ab6752f019518b4a2cb0724bd027755f
>
> Git release tag was created:
> https://gitbox.apache.org/repos/asf?p=ignite-python-thin-client.git;a=tag;h=refs/tags/0.5.0.rc1
>
> The vote is formal, see voting guidelines
> https://www.apache.org/foundation/voting.html
>
> +1 - to accept pyignite-0.5.0-rc1
> 0 - don't care either way
> -1 - DO NOT accept pyignite-0.5.0-rc1


[VOTE] Release pyignite 0.5.0-rc1

2021-06-16 Thread Ivan Daschinsky
Dear Igniters!

Release candidate binaries for subj are uploaded and ready for vote
You can find them here:
https://dist.apache.org/repos/dist/dev/ignite/pyignite/0.5.0-rc1

If you follow the link above, you will find source package (*.tar.gz and *.zip)
and binary packages (wheels) for windows (amd64) and linux (x86_64)
for pythons 36, 37, 38, 39. Also, there are sha512 and gpg signatures.
Code signing keys can be found here -- https://downloads.apache.org/ignite/KEYS
Here you can find instructions how to verify packages
https://www.apache.org/info/verification.html

You can install binary package for specific version of python using pip
For example do this on linux for python 3.8
>> pip install pyignite-0.5.0-cp38-cp38-manylinux1_x86_64.whl

You can build and install package from source using this command:
>> pip install pyignite-0.5.0.tar.gz
You can build wheel on your platform using this command:
>> pip wheel --no-deps pyignite-0.5.0.tar.gz

For building C module, you should have python headers and C compiler installed.
(i.e. for ubuntu sudo apt install build-essential python3-dev)
In Mac OS X xcode-tools and python from homebrew are the best option.

In order to check whether C module works, use following:
>> from pyignite import _cutils
>> print(_cutils.hashcode('test'))
>> 3556498

You can find documentation here:
https://apache-ignite-binary-protocol-client.readthedocs.io/en/0.5.0.rc1

You can find examples here (to check them, you should start ignite locally):
https://apache-ignite-binary-protocol-client.readthedocs.io/en/0.5.0.rc1/examples.html
Also, examples can be found in source archive in examples subfolder.
docker-compose.yml is supplied in order to start ignite quickly. (Use
`docker-compose up -d` to start 3 nodes cluster and `docker-compose
down` to shut down it)

Release notes:
https://gitbox.apache.org/repos/asf?p=ignite-python-thin-client.git;a=blob;f=RELEASE_NOTES.txt;h=9d2ae81af2de22ce9e8c9d3b7ece14dd9e75ca0e;hb=61c83cb0ab6752f019518b4a2cb0724bd027755f

Git release tag was created:
https://gitbox.apache.org/repos/asf?p=ignite-python-thin-client.git;a=tag;h=refs/tags/0.5.0.rc1

The vote is formal, see voting guidelines
https://www.apache.org/foundation/voting.html

+1 - to accept pyignite-0.5.0-rc1
0 - don't care either way
-1 - DO NOT accept pyignite-0.5.0-rc1


[CANCEL][VOTE] Release pyignite 0.5.0-rc0

2021-06-16 Thread Ivan Daschinsky
This vote was canceled since it was decided to add [1] to the release.
[1] has been already merged to master branch and cherrypicked to
release branch. New vote will be created soon.

[1]  -- https://issues.apache.org/jira/browse/IGNITE-14911


Re: [DISCUSSION] Add cache statistics switch to control script

2021-06-16 Thread Andrey Gura
Hi, Ilya

Could you please provide more details about how it should work (in
issue description)? E.g. what if I pass the cache group name instead
of the name of a particular cache? What if I will not pass any cache
names?

Also we don't use the word "statistics" anymore (while it is still
called statistics in configuration). What about the word "metrics"?

On Wed, Jun 16, 2021 at 2:31 PM Shishkov Ilya  wrote:
>
> Dear Ignite Community!
> I propose to discuss the addition of a new feature for control script:
> cache statistics switch (enabling / disabling). Currently, cache statistics
> can be toggled only via IgniteVisorCmd or JMX.
> I've created the corresponding issue [1] and suggest adding an extra option
> to a '--cache' command of the control script, eg.:
>
> --cache statistics enable|disable [cacheName1,...,cacheNameN]
>
> Please, tell me if you have any suggestions or remarks against this feature.
>
> 1. https://issues.apache.org/jira/browse/IGNITE-14913


Re: [VOTE] Release pyignite 0.5.0-rc0

2021-06-16 Thread Ivan Daschinsky
Yes, I agree, after merging
https://issues.apache.org/jira/browse/IGNITE-14911 that resolves this
issue, I suggests to start a new vote.

ср, 16 июн. 2021 г. в 15:54, Igor Sapego :
>
> I propose to cancel this release and fix the issue which was highlighted in
> the
> "Seconds and milliseconds confusion in python thin client" thread.
>
> WDYT?
>
> Best Regards,
> Igor
>
>
> On Wed, Jun 16, 2021 at 12:10 AM Igor Sapego  wrote:
>
> > +1 from me
> >
> > Uploaded to test.pipy.org: https://test.pypi.org/project/pyignite/0.5.0/
> >
> > Everything looks good.
> >
> > Best Regards,
> > Igor
> >
> >
> > On Tue, Jun 15, 2021 at 10:09 PM Ivan Daschinsky 
> > wrote:
> >
> >> Also checked hash sums and signature. Packages are verified and
> >> signature is OK, signed by Igor Sapego with key
> >> 5C10A0722D947727923C98B5AF35DBD958FE8DC5
> >> Keys can be found here -- https://downloads.apache.org/ignite/KEYS
> >> Here you can find instructions how to verify packages
> >> https://www.apache.org/info/verification.html
> >>
> >> вт, 15 июн. 2021 г. в 22:01, Ivan Daschinsky :
> >> >
> >> > +1 From me
> >> > Checked building from source on Ubuntu 20.04 amd64 for pythons 3.6.12
> >> > 3.7.9 3.8.6 3.9.1.
> >> > Checked installing binary packages on Ubuntu 20.04 amd64 and Windows
> >> > 10 amd 64 for pythons 3.6.12 3.7.9 3.8.6 3.9.1.
> >> > Run examples for both platforms
> >> > Check that native module works
> >> > Checked documentation for tag 0.5.0.rc0 on readthedocs.io
> >> >
> >> > вт, 15 июн. 2021 г. в 21:58, Ivan Daschinsky :
> >> > >
> >> > > Dear Igniters!
> >> > >
> >> > > Release candidate binaries for subj are uploaded and ready for vote
> >> > > You can find them here:
> >> > > https://dist.apache.org/repos/dist/dev/ignite/pyignite/0.5.0-rc0
> >> > >
> >> > > If you follow the link above, you will find source package (*.tar.gz
> >> and *.zip)
> >> > > and binary packages (wheels) for windows (amd64) and linux (x86_64)
> >> > > for pythons 36, 37, 38, 39. Also, there are sha512 and gpg signatures.
> >> > >
> >> > > You can install binary package for specific version of python using
> >> pip
> >> > > For example do this on linux for python 3.8
> >> > > >> pip install pyignite-0.4.0-cp38-cp38-manylinux1_x86_64.whl
> >> > >
> >> > > You can build and install package from source using this command:
> >> > > >> pip install pyignite-0.4.0.tar.gz
> >> > > You can build wheel on your platform using this command:
> >> > > >> pip wheel --no-deps pyignite-0.4.0.tar.gz
> >> > >
> >> > > For building C module, you should have python headers and C compiler
> >> installed.
> >> > > (i.e. for ubuntu sudo apt install build-essential python3-dev)
> >> > > In Mac OS X xcode-tools and python from homebrew are the best option.
> >> > >
> >> > > In order to check whether C module works, use following:
> >> > > >> from pyignite import _cutils
> >> > > >> print(_cutils.hashcode('test'))
> >> > > >> 3556498
> >> > >
> >> > > You can find documentation here:
> >> > >
> >> https://apache-ignite-binary-protocol-client.readthedocs.io/en/0.5.0.rc0
> >> > >
> >> > > You can find examples here (to check them, you should start ignite
> >> locally):
> >> > >
> >> https://apache-ignite-binary-protocol-client.readthedocs.io/en/0.5.0.rc0/examples.html
> >> > > Also, examples can be found in source archive in examples subfolder.
> >> > > docker-compose.yml is supplied in order to start ignite quickly. (Use
> >> > > `docker-compose up -d` to start 3 nodes cluster and `docker-compose
> >> > > down` to shut down it)
> >> > >
> >> > > Release notes:
> >> > >
> >> https://gitbox.apache.org/repos/asf?p=ignite-python-thin-client.git;a=blob;f=RELEASE_NOTES.txt;h=9d2ae81af2de22ce9e8c9d3b7ece14dd9e75ca0e;hb=61c83cb0ab6752f019518b4a2cb0724bd027755f
> >> > >
> >> > > Git release tag was created:
> >> > >
> >> https://gitbox.apache.org/repos/asf?p=ignite-python-thin-client.git;a=tag;h=refs/tags/0.5.0.rc0
> >> > >
> >> > > The vote is formal, see voting guidelines
> >> > > https://www.apache.org/foundation/voting.html
> >> > >
> >> > > +1 - to accept pyignite-0.5.0-rc0
> >> > > 0 - don't care either way
> >> > > -1 - DO NOT accept pyignite-0.5.0-rc0
> >> > >
> >> > > The vote will end at June, 17 15:00 UTC.
> >> >
> >> >
> >> >
> >> > --
> >> > Sincerely yours, Ivan Daschinskiy
> >>
> >>
> >>
> >> --
> >> Sincerely yours, Ivan Daschinskiy
> >>
> >



-- 
Sincerely yours, Ivan Daschinskiy


Re: [DISCUSSION] Code style. Variable abbrevations

2021-06-16 Thread Andrey Gura
Hi,

I understand that this rule seems too strict or may be weird. But
removing the rule could lead to review comments like "use future
instead of fut". So my proposal is to change rule from "required" to
"recommended".

On Wed, Jun 16, 2021 at 2:49 PM Valentin Kulichenko
 wrote:
>
> Konstantin, thanks for chiming in.
>
> That's exactly my concern. Overformalization is generally not a good thing.
> Someone used "mess" to abbreviate "message"? That is surely not a good
> coding style, but that's what code reviews are for. I believe that our
> committers are more than capable of identifying such issues.
>
> At the same time, with the current rules, we are *forced* to use
> abbreviations like "locAddrGrpMgr", whether we like it or not. All it does
> is makes it harder to contribute to Ignite, without providing any clear
> value.
>
> -Val
>
> On Thu, Jun 10, 2021 at 9:46 AM Konstantin Orlov 
> wrote:
>
> > +1 for making this optional
> >
> > Common abbreviation rules is good for sure, but sometimes I getting sick
> > of all those locAddrGrpMgr.
> >
> >
> > --
> > Regards,
> > Konstantin Orlov
> >
> >
> >
> >
> > > On 7 Jun 2021, at 14:31, Nikolay Izhikov  wrote:
> > >
> > > Hello, Anton, Alexei.
> > >
> > > Thanks for the feedback.
> > >
> > > Personally, I’m pretty happy current abbreviation rules too.
> > > Let see what we can do to make our codebase even more consistent.
> > >
> > >> 7 июня 2021 г., в 13:23, Alexei Scherbakov <
> > alexey.scherbak...@gmail.com> написал(а):
> > >>
> > >> -1
> > >> Common abbrevs add quality to the code.
> > >>
> > >> пн, 7 июн. 2021 г. в 12:38, Anton Vinogradov :
> > >>
> > >>> -1 here.
> > >>> We can fix the code and set up the rule.
> > >>>
> > >>> This will help to prevent having a weird abbreviation like "mess" (from
> > >>> "message") or "ign" (from "Ignite").
> > >>> Also, the abbreviations list (hardcoded at IDEA plugin) allows to have
> > same
> > >>> names across the whole code, this should simplify the reading.
> > >>>
> > >>> On Sat, Jun 5, 2021 at 10:49 PM Valentin Kulichenko <
> > >>> valentin.kuliche...@gmail.com> wrote:
> > >>>
> >  I also support removing this requirement. It’s not the first time
> > someone
> >  brings this up, and so far we haven’t been able to fix it. Not worth
> > it
> > >>> in
> >  my view.
> > 
> >  -Val
> > 
> >  On Sat, Jun 5, 2021 at 11:54 AM Nikolay Izhikov 
> >  wrote:
> > 
> > > Hello, guys.
> > >
> > > Thanks for the feedback.
> > >
> > > Dmitry,
> > >
> > >> Manual rule enforcement saves a couple of symbols in code
> > >
> > > I’m talking about automatic check.
> > > We can implement it easily.
> > > So, if we decide to keep this rule all we need is to fix current
> > > violations (several thousand!).
> > > After it, checkstyle will automatically enforce this rule.
> > > As you may know, currently, any PR checked according to our
> > checkstyle
> > > rules.
> > > Please, take a look at little green check sign after PR name.
> > >
> > >
> > >> 5 июня 2021 г., в 00:57, Dmitry Pavlov 
> >  написал(а):
> > >>
> > >> +1 for removal. Cnt and count both seem to be fine.
> > >>
> > >> Manual rule enforcement saves a couple of symbols in code, but
> > >>> requires
> > > some time from both contributor and reviewer.
> > >>
> > >> Sincerely,
> > >> Dmitriy Pavlov
> > >>
> > >> On 2021/06/04 19:18:37, Pavel Tupitsyn 
> > wrote:
> > >>> In my opinion, we should remove this rule.
> > >>> Looks like a waste of time. freq or frequency, cnt or count, it is
> >  fine
> > >>> either way.
> > >>>
> > >>> On Fri, Jun 4, 2021 at 7:39 PM Nikolay Izhikov <
> > nizhi...@apache.org
> > 
> > > wrote:
> > >>>
> >  Hello, Igniters.
> > 
> >  Right now, we have the rule to use some predefined list of
> >  abbrevation
> > > for
> >  variable names [1].
> >  Some of the reviewers ask to follow this rule strictly.
> > 
> > > It is required to use abbreviated form for code consistency.
> > 
> >  I tried to implement this rule in form of checkstyle check [2] and
> > >>> it
> >  seems like many of use doesn’t follow this requirement.
> >  My check found 4124 violation in core module.
> > 
> >  Should we make this rule optional in documentation or should we
> >  remove
> > > it
> >  completely?
> >  Or should someone proceed and fix all the violations?
> > 
> >  WDYT?
> > 
> > 
> >  Example of output:
> >  ```
> >  [ERROR]
> > 
> > >
> > 
> > >>>
> > /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsWithTtlTest.java:94:
> >  Abbrevation should be used for CACHE_NAME_LOCAL_ATOMIC! 

Re: [VOTE] Release pyignite 0.5.0-rc0

2021-06-16 Thread Igor Sapego
I propose to cancel this release and fix the issue which was highlighted in
the
"Seconds and milliseconds confusion in python thin client" thread.

WDYT?

Best Regards,
Igor


On Wed, Jun 16, 2021 at 12:10 AM Igor Sapego  wrote:

> +1 from me
>
> Uploaded to test.pipy.org: https://test.pypi.org/project/pyignite/0.5.0/
>
> Everything looks good.
>
> Best Regards,
> Igor
>
>
> On Tue, Jun 15, 2021 at 10:09 PM Ivan Daschinsky 
> wrote:
>
>> Also checked hash sums and signature. Packages are verified and
>> signature is OK, signed by Igor Sapego with key
>> 5C10A0722D947727923C98B5AF35DBD958FE8DC5
>> Keys can be found here -- https://downloads.apache.org/ignite/KEYS
>> Here you can find instructions how to verify packages
>> https://www.apache.org/info/verification.html
>>
>> вт, 15 июн. 2021 г. в 22:01, Ivan Daschinsky :
>> >
>> > +1 From me
>> > Checked building from source on Ubuntu 20.04 amd64 for pythons 3.6.12
>> > 3.7.9 3.8.6 3.9.1.
>> > Checked installing binary packages on Ubuntu 20.04 amd64 and Windows
>> > 10 amd 64 for pythons 3.6.12 3.7.9 3.8.6 3.9.1.
>> > Run examples for both platforms
>> > Check that native module works
>> > Checked documentation for tag 0.5.0.rc0 on readthedocs.io
>> >
>> > вт, 15 июн. 2021 г. в 21:58, Ivan Daschinsky :
>> > >
>> > > Dear Igniters!
>> > >
>> > > Release candidate binaries for subj are uploaded and ready for vote
>> > > You can find them here:
>> > > https://dist.apache.org/repos/dist/dev/ignite/pyignite/0.5.0-rc0
>> > >
>> > > If you follow the link above, you will find source package (*.tar.gz
>> and *.zip)
>> > > and binary packages (wheels) for windows (amd64) and linux (x86_64)
>> > > for pythons 36, 37, 38, 39. Also, there are sha512 and gpg signatures.
>> > >
>> > > You can install binary package for specific version of python using
>> pip
>> > > For example do this on linux for python 3.8
>> > > >> pip install pyignite-0.4.0-cp38-cp38-manylinux1_x86_64.whl
>> > >
>> > > You can build and install package from source using this command:
>> > > >> pip install pyignite-0.4.0.tar.gz
>> > > You can build wheel on your platform using this command:
>> > > >> pip wheel --no-deps pyignite-0.4.0.tar.gz
>> > >
>> > > For building C module, you should have python headers and C compiler
>> installed.
>> > > (i.e. for ubuntu sudo apt install build-essential python3-dev)
>> > > In Mac OS X xcode-tools and python from homebrew are the best option.
>> > >
>> > > In order to check whether C module works, use following:
>> > > >> from pyignite import _cutils
>> > > >> print(_cutils.hashcode('test'))
>> > > >> 3556498
>> > >
>> > > You can find documentation here:
>> > >
>> https://apache-ignite-binary-protocol-client.readthedocs.io/en/0.5.0.rc0
>> > >
>> > > You can find examples here (to check them, you should start ignite
>> locally):
>> > >
>> https://apache-ignite-binary-protocol-client.readthedocs.io/en/0.5.0.rc0/examples.html
>> > > Also, examples can be found in source archive in examples subfolder.
>> > > docker-compose.yml is supplied in order to start ignite quickly. (Use
>> > > `docker-compose up -d` to start 3 nodes cluster and `docker-compose
>> > > down` to shut down it)
>> > >
>> > > Release notes:
>> > >
>> https://gitbox.apache.org/repos/asf?p=ignite-python-thin-client.git;a=blob;f=RELEASE_NOTES.txt;h=9d2ae81af2de22ce9e8c9d3b7ece14dd9e75ca0e;hb=61c83cb0ab6752f019518b4a2cb0724bd027755f
>> > >
>> > > Git release tag was created:
>> > >
>> https://gitbox.apache.org/repos/asf?p=ignite-python-thin-client.git;a=tag;h=refs/tags/0.5.0.rc0
>> > >
>> > > The vote is formal, see voting guidelines
>> > > https://www.apache.org/foundation/voting.html
>> > >
>> > > +1 - to accept pyignite-0.5.0-rc0
>> > > 0 - don't care either way
>> > > -1 - DO NOT accept pyignite-0.5.0-rc0
>> > >
>> > > The vote will end at June, 17 15:00 UTC.
>> >
>> >
>> >
>> > --
>> > Sincerely yours, Ivan Daschinskiy
>>
>>
>>
>> --
>> Sincerely yours, Ivan Daschinskiy
>>
>


Re: [DISCUSSION] Code style. Variable abbrevations

2021-06-16 Thread Valentin Kulichenko
Konstantin, thanks for chiming in.

That's exactly my concern. Overformalization is generally not a good thing.
Someone used "mess" to abbreviate "message"? That is surely not a good
coding style, but that's what code reviews are for. I believe that our
committers are more than capable of identifying such issues.

At the same time, with the current rules, we are *forced* to use
abbreviations like "locAddrGrpMgr", whether we like it or not. All it does
is makes it harder to contribute to Ignite, without providing any clear
value.

-Val

On Thu, Jun 10, 2021 at 9:46 AM Konstantin Orlov 
wrote:

> +1 for making this optional
>
> Common abbreviation rules is good for sure, but sometimes I getting sick
> of all those locAddrGrpMgr.
>
>
> --
> Regards,
> Konstantin Orlov
>
>
>
>
> > On 7 Jun 2021, at 14:31, Nikolay Izhikov  wrote:
> >
> > Hello, Anton, Alexei.
> >
> > Thanks for the feedback.
> >
> > Personally, I’m pretty happy current abbreviation rules too.
> > Let see what we can do to make our codebase even more consistent.
> >
> >> 7 июня 2021 г., в 13:23, Alexei Scherbakov <
> alexey.scherbak...@gmail.com> написал(а):
> >>
> >> -1
> >> Common abbrevs add quality to the code.
> >>
> >> пн, 7 июн. 2021 г. в 12:38, Anton Vinogradov :
> >>
> >>> -1 here.
> >>> We can fix the code and set up the rule.
> >>>
> >>> This will help to prevent having a weird abbreviation like "mess" (from
> >>> "message") or "ign" (from "Ignite").
> >>> Also, the abbreviations list (hardcoded at IDEA plugin) allows to have
> same
> >>> names across the whole code, this should simplify the reading.
> >>>
> >>> On Sat, Jun 5, 2021 at 10:49 PM Valentin Kulichenko <
> >>> valentin.kuliche...@gmail.com> wrote:
> >>>
>  I also support removing this requirement. It’s not the first time
> someone
>  brings this up, and so far we haven’t been able to fix it. Not worth
> it
> >>> in
>  my view.
> 
>  -Val
> 
>  On Sat, Jun 5, 2021 at 11:54 AM Nikolay Izhikov 
>  wrote:
> 
> > Hello, guys.
> >
> > Thanks for the feedback.
> >
> > Dmitry,
> >
> >> Manual rule enforcement saves a couple of symbols in code
> >
> > I’m talking about automatic check.
> > We can implement it easily.
> > So, if we decide to keep this rule all we need is to fix current
> > violations (several thousand!).
> > After it, checkstyle will automatically enforce this rule.
> > As you may know, currently, any PR checked according to our
> checkstyle
> > rules.
> > Please, take a look at little green check sign after PR name.
> >
> >
> >> 5 июня 2021 г., в 00:57, Dmitry Pavlov 
>  написал(а):
> >>
> >> +1 for removal. Cnt and count both seem to be fine.
> >>
> >> Manual rule enforcement saves a couple of symbols in code, but
> >>> requires
> > some time from both contributor and reviewer.
> >>
> >> Sincerely,
> >> Dmitriy Pavlov
> >>
> >> On 2021/06/04 19:18:37, Pavel Tupitsyn 
> wrote:
> >>> In my opinion, we should remove this rule.
> >>> Looks like a waste of time. freq or frequency, cnt or count, it is
>  fine
> >>> either way.
> >>>
> >>> On Fri, Jun 4, 2021 at 7:39 PM Nikolay Izhikov <
> nizhi...@apache.org
> 
> > wrote:
> >>>
>  Hello, Igniters.
> 
>  Right now, we have the rule to use some predefined list of
>  abbrevation
> > for
>  variable names [1].
>  Some of the reviewers ask to follow this rule strictly.
> 
> > It is required to use abbreviated form for code consistency.
> 
>  I tried to implement this rule in form of checkstyle check [2] and
> >>> it
>  seems like many of use doesn’t follow this requirement.
>  My check found 4124 violation in core module.
> 
>  Should we make this rule optional in documentation or should we
>  remove
> > it
>  completely?
>  Or should someone proceed and fix all the violations?
> 
>  WDYT?
> 
> 
>  Example of output:
>  ```
>  [ERROR]
> 
> >
> 
> >>>
> /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsWithTtlTest.java:94:
>  Abbrevation should be used for CACHE_NAME_LOCAL_ATOMIC! Please,
> use
> > loc,
>  instead of LOCAL [IgniteAbbrevationsRule]
>  [ERROR]
> 
> >
> 
> >>>
> /Users/sbt-izhikov-nv/work/ignite/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsWithTtlTest.java:97:
>  Abbrevation should be used for CACHE_NAME_LOCAL_TX! Please, use
> >>> loc,
>  instead of LOCAL [IgniteAbbrevationsRule]
>  [ERROR]
> 
> >
> 
> >>>
> 

Re: [DISCUSSION] Javadoc style requirements and checks in Ignite-3.

2021-06-16 Thread Andrey Gura
Hi,

I think that scope should be limited by public API  for beginning.
Also I don't sure that we should support specific tags like @apiNote,
@implSpec, @implNote.

Let's move using the following plan:

1. Create an empty (effectively) checkstyle config for javadoc
checking and commit it to the repository. After this step it will be
possible to configure TC in order to use this configuration.
2. Configure TC.
3. Commit a non-empty checkstyle config, but all modules should be
excluded from checking on this step.
4. For each module create a JIRA issue. Module maintainers fix
corresponding tickets and remove module exclusion from checking.
5. Add information about javadoc validation targets to DEVNOTES.md
file (could be done on step 3)

Any objections?

On Tue, Apr 20, 2021 at 11:40 AM Andrey Mashenkov
 wrote:
>
> I've fixed the PR.
>
> Now,
> 1. Javadoc style is only checked if it is present for the element. All
> declared javadoc tags MUST have a description.
> So, one should either write correct javadoc or don't write at all.
> 2. Additional checks performed for non-internal and non-test classes. All
> public and protected elements must have non-emtpy javadoc.
>
> So, checkstyle detects 500+ missed descriptions (missed javadocs, tags
> descriptions, tags themselves) in public scope
> and 180+ bad-styles (missed brased, spaces, empty-lines) javadocs in whole
> codebase.
>
> I'd suggest to force non-empty javadocs for all non-test classes including
> internal (but their members) as well.
>
>
>
> On Mon, Apr 19, 2021 at 6:37 PM Andrey Mashenkov 
> wrote:
>
> > Alexey,
> >
> > These are bad examples and unfortunately, most of the Ignite code doesn't
> > look self-descriptive.
> > (Do you feel the difference between @SerializableTransient and
> > @TransientSerializable?)
> >
> > To understand the semantic of e.g. 'metricsHistSize' you have to analyze
> > its usages.
> > Getter and setter for this property look better, but it still not clear
> > what happens with metric values above the limit?
> >
> > I have another example, one of the core components GridDhtPartitionDemander
> > has totally useless javadoc.
> > It is obviously has nothing with thread pool + "local cache" phrase looks
> > very confusing.
> >
> > /**
> >  * Thread pool for requesting partitions from other nodes and populating 
> > local cache.
> >  */
> > public class GridDhtPartitionDemander
> >
> > To understand the purpose of this internal component I have to analyze its
> > code and usages.
> > However, if one will face unexpected behavior, he/she may be confused if
> > it is a lack of javadoc or wrong behavior.
> >
> > There is no way to restrict or avoid such issues with any checks.
> > Agree, meaningful javadocs as self-descriptive code is more about culture
> > and discipline.
> >
> > The absence of javadoc on internal API, unreasonably raises the entry
> > threshold for new developers and makes the development of intra-component
> > interaction harder.
> > I admit a high-level description for public classes may be enough to
> > resolve this without pushing developers to write empty/useless docs for
> > every method/field.
> >
> >
> > On Mon, Apr 19, 2021 at 5:21 PM Alexey Kukushkin <
> > kukushkinale...@gmail.com> wrote:
> >
> >> I personally do not understand why we need mandatory javadoc even in
> >> public
> >> modules. I think javadoc is needed only when the code is not
> >> self-descriptive. What value does a javadoc like this bring
> >> <
> >> https://github.com/apache/ignite/blob/2.10.0/modules/core/src/main/java/org/apache/ignite/configuration/IgniteConfiguration.java
> >> >
> >> to a developer:
> >>
> >> /** Default metrics history size (value is {@code 1}). */
> >> public static final int DFLT_METRICS_HISTORY_SIZE = 1;
> >>
> >> /** Metrics history time. */
> >> private int metricsHistSize = DFLT_METRICS_HISTORY_SIZE;
> >>
> >> BTW, I picked a random example and it already has a copy-paste mistake in
> >> the javadoc, which is there for years. I think that indicates it has no
> >> real value and developers just do it to comply with the rule.
> >>
> >> Some say mandatory javadoc is for the discipline but I think Apache Ignite
> >> developers are mature enough to understand when additional documentation
> >> is
> >> really required.
> >>
> >>
> >>
> >> пн, 19 апр. 2021 г. в 16:37, Ivan Pavlukhin :
> >>
> >> > Hi Andrey and Igniters,
> >> >
> >> > Sorry if I misread something but I have totally different opinion in
> >> > one aspect. In my mind it is much better in practice to follow strict
> >> > rules for public API javadocs but not for internals. I would use
> >> > static checks for API packages and disable them for internal ones and
> >> > refine code readability during code review. Main motivation here is
> >> > that ubiquitous javadocs did not work well in ignite-2 and I believe
> >> > it would not in ignite-3.
> >> >
> >> > 2021-04-19 13:30 GMT+03:00, Andrey Mashenkov <
> >> andrey.mashen...@gmail.com>:
> >> > > Hi 

[DISCUSSION] Add cache statistics switch to control script

2021-06-16 Thread Shishkov Ilya
Dear Ignite Community!
I propose to discuss the addition of a new feature for control script:
cache statistics switch (enabling / disabling). Currently, cache statistics
can be toggled only via IgniteVisorCmd or JMX.
I've created the corresponding issue [1] and suggest adding an extra option
to a '--cache' command of the control script, eg.:

--cache statistics enable|disable [cacheName1,...,cacheNameN]

Please, tell me if you have any suggestions or remarks against this feature.

1. https://issues.apache.org/jira/browse/IGNITE-14913


Re: Seconds and milliseconds confusion in python thin client

2021-06-16 Thread Ivan Daschinsky
I have a compromise variant.

1. Large timeouts are set usually only for expire_policy. I suggest to
support datetime.timedelta here and int as milliseconds
2. All others timeouts should accept only ints as milliseconds.
3. Only timeout in Connection as sockettimeout should remain float in
seconds, because it corresponds to socket.settimeout method in python
(accepts floating point number)

This is a compromise between usability and backward compatibility

WDYT?

ср, 16 июн. 2021 г. в 11:09, Ivan Daschinsky :
>
> I've created ticket for it https://issues.apache.org/jira/browse/IGNITE-14911
>
> ср, 16 июн. 2021 г. в 08:37, Ivan Daschinsky :
> >
> > Ops, i don't even know about it. I believe that this is so rarely used, i 
> > don't even noticed it. I am talking about transactions and expiry policy. I 
> > suppose that in the case of sql we can simply change it.
> >
> > ср, 16 июн. 2021 г., 00:46 Igor Sapego :
> >>
> >> Why is it not released?
> >>
> >> I can see client.sql(timeout) in 0.4.0 for example, which is int number of
> >> ms.
> >>
> >> Best Regards,
> >> Igor
> >>
> >>
> >> On Tue, Jun 15, 2021 at 11:52 PM Ivan Daschinsky 
> >> wrote:
> >>
> >> > BTW, common approach is to treat both ints and floats as seconds. Floats
> >> > are used to set timeout with millisecods precision. I.e. 
> >> > asyncio.sleep(1.0)
> >> > and asyncio.sleep(1) pauses coroutine for 1 sec. Lets create ticket for 
> >> > it,
> >> > stop voting for 0.5.0.rc0 and schedule next vote.
> >> >
> >> > вт, 15 июн. 2021 г., 23:49 Ivan Daschinsky :
> >> >
> >> > > Igor, I suppose that you are probably right. But there is no need to
> >> > > notice or deprecate something. This functionality is not released yet
> >> > >
> >> > > вт, 15 июн. 2021 г., 23:41 Igor Sapego :
> >> > >
> >> > >> Hi Igniters,
> >> > >>
> >> > >> I've noticed a weird behaviour of python thin client. In those places
> >> > >> where
> >> > >> we have
> >> > >> timeouts or any other parameters that take time in some places we 
> >> > >> treat
> >> > it
> >> > >> like integer
> >> > >> number of milliseconds, in others it can take both floats (as a number
> >> > of
> >> > >> seconds)
> >> > >> and ints (number of milliseconds). This approach looks very confusing 
> >> > >> to
> >> > >> me
> >> > >> as
> >> > >> it leads to things where tx_start(1) and tx_start(1.0) are not 
> >> > >> actually
> >> > >> the
> >> > >> same thing.
> >> > >>
> >> > >> AFAIK in python the most common way to pass time to such functions is 
> >> > >> to
> >> > >> use floats
> >> > >> as a number of seconds. This is the approach I propose to use in our 
> >> > >> API
> >> > >> as
> >> > >> well. Let's
> >> > >> deprecate usage of ints in those functions with the appropriate 
> >> > >> warning
> >> > >> before getting rid
> >> > >> of it.
> >> > >>
> >> > >> What do you think?
> >> > >>
> >> > >> Best Regards,
> >> > >> Igor
> >> > >>
> >> > >
> >> >
>
>
>
> --
> Sincerely yours, Ivan Daschinskiy



-- 
Sincerely yours, Ivan Daschinskiy


Re: Seconds and milliseconds confusion in python thin client

2021-06-16 Thread Ivan Daschinsky
I've created ticket for it https://issues.apache.org/jira/browse/IGNITE-14911

ср, 16 июн. 2021 г. в 08:37, Ivan Daschinsky :
>
> Ops, i don't even know about it. I believe that this is so rarely used, i 
> don't even noticed it. I am talking about transactions and expiry policy. I 
> suppose that in the case of sql we can simply change it.
>
> ср, 16 июн. 2021 г., 00:46 Igor Sapego :
>>
>> Why is it not released?
>>
>> I can see client.sql(timeout) in 0.4.0 for example, which is int number of
>> ms.
>>
>> Best Regards,
>> Igor
>>
>>
>> On Tue, Jun 15, 2021 at 11:52 PM Ivan Daschinsky 
>> wrote:
>>
>> > BTW, common approach is to treat both ints and floats as seconds. Floats
>> > are used to set timeout with millisecods precision. I.e. asyncio.sleep(1.0)
>> > and asyncio.sleep(1) pauses coroutine for 1 sec. Lets create ticket for it,
>> > stop voting for 0.5.0.rc0 and schedule next vote.
>> >
>> > вт, 15 июн. 2021 г., 23:49 Ivan Daschinsky :
>> >
>> > > Igor, I suppose that you are probably right. But there is no need to
>> > > notice or deprecate something. This functionality is not released yet
>> > >
>> > > вт, 15 июн. 2021 г., 23:41 Igor Sapego :
>> > >
>> > >> Hi Igniters,
>> > >>
>> > >> I've noticed a weird behaviour of python thin client. In those places
>> > >> where
>> > >> we have
>> > >> timeouts or any other parameters that take time in some places we treat
>> > it
>> > >> like integer
>> > >> number of milliseconds, in others it can take both floats (as a number
>> > of
>> > >> seconds)
>> > >> and ints (number of milliseconds). This approach looks very confusing to
>> > >> me
>> > >> as
>> > >> it leads to things where tx_start(1) and tx_start(1.0) are not actually
>> > >> the
>> > >> same thing.
>> > >>
>> > >> AFAIK in python the most common way to pass time to such functions is to
>> > >> use floats
>> > >> as a number of seconds. This is the approach I propose to use in our API
>> > >> as
>> > >> well. Let's
>> > >> deprecate usage of ints in those functions with the appropriate warning
>> > >> before getting rid
>> > >> of it.
>> > >>
>> > >> What do you think?
>> > >>
>> > >> Best Regards,
>> > >> Igor
>> > >>
>> > >
>> >



-- 
Sincerely yours, Ivan Daschinskiy