IGNITE-3999 review

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

IGNITE-3999 review

Amir Akhmedov
Hi Team,

Can anybody review https://github.com/apache/ignite/pull/3444 please?

Thanks,
Amir
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Dmitriy Pavlov
Hi Amir,

Thank you for your contribution.

Could you share link to Teamcity run all?

Sincerely
Dmitry Pavlov

ср, 21 мар. 2018 г., 21:23 Amir Akhmedov <[hidden email]>:

> Hi Team,
>
> Can anybody review https://github.com/apache/ignite/pull/3444 please?
>
> Thanks,
> Amir
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Amir Akhmedov
Dmitry,

Since the last TC test run has been done almost 2 months ago I decided to
run it again since lot of changes were introduced.
I will let you know when tests are finished.

Thanks,
Amir
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Amir Akhmedov
Dmitry,

TeamCity RunAll completed but the results look awkward. Almost all test
suites marked as failed. I noticed the same behavior for other builds
though.
If you don't mind could you please check is it alright and let me know?

https://ci.ignite.apache.org/viewLog.html?buildTypeId=IgniteTests24Java8_RunAll&buildId=1150589&branch_IgniteTests24Java8_RunAll=pull/3444/head

Thanks,
Amir
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Dmitriy Pavlov
Hi Amir,

It seems reason was compilation error
 Compilation error: Compiler
Compilation failure
/data/teamcity/work/bd85361428dcdb1/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlCaseInsensitiveTest.java:[614,23]
cannot find symbol
  symbol:   method
querySqlFieldsNoCache(org.apache.ignite.cache.query.SqlFieldsQuery,boolean)
  location: variable qryProc of type
org.apache.ignite.internal.processors.query.GridQueryProcessor

Sincerely,
Dmitriy Pavlov

чт, 22 мар. 2018 г. в 6:02, Amir Akhmedov <[hidden email]>:

> Dmitry,
>
> TeamCity RunAll completed but the results look awkward. Almost all test
> suites marked as failed. I noticed the same behavior for other builds
> though.
> If you don't mind could you please check is it alright and let me know?
>
>
> https://ci.ignite.apache.org/viewLog.html?buildTypeId=IgniteTests24Java8_RunAll&buildId=1150589&branch_IgniteTests24Java8_RunAll=pull/3444/head
>
> Thanks,
> Amir
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Amir Akhmedov
Dmitriy,

Thank you for a hint. Please find below a link to new TeamCity run. This
time looks much better but still has a lot of failed tests. But most of
them not related to my changes except failed .Net tests. Can you please
review and advise on next steps?

Thanks,
Amir

https://ci.ignite.apache.org/viewLog.html?buildId=1152609&tab=buildResultsDiv&buildTypeId=IgniteTests24Java8_RunAll

On Thu, Mar 22, 2018 at 7:36 AM, Dmitry Pavlov <[hidden email]>
wrote:

> Hi Amir,
>
> It seems reason was compilation error
>  Compilation error: Compiler
> Compilation failure
> /data/teamcity/work/bd85361428dcdb1/modules/indexing/src/test/java/org/
> apache/ignite/internal/processors/query/IgniteSqlCaseInsensitiveTest.
> java:[614,23]
> cannot find symbol
>   symbol:   method
> querySqlFieldsNoCache(org.apache.ignite.cache.query.
> SqlFieldsQuery,boolean)
>   location: variable qryProc of type
> org.apache.ignite.internal.processors.query.GridQueryProcessor
>
> Sincerely,
> Dmitriy Pavlov
>
> чт, 22 мар. 2018 г. в 6:02, Amir Akhmedov <[hidden email]>:
>
> > Dmitry,
> >
> > TeamCity RunAll completed but the results look awkward. Almost all test
> > suites marked as failed. I noticed the same behavior for other builds
> > though.
> > If you don't mind could you please check is it alright and let me know?
> >
> >
> > https://ci.ignite.apache.org/viewLog.html?buildTypeId=
> IgniteTests24Java8_RunAll&buildId=1150589&branch_
> IgniteTests24Java8_RunAll=pull/3444/head
> >
> > Thanks,
> > Amir
> >
>



--
Sincerely Yours Amir Akhmedov
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Alexey Kuznetsov
Vladimir,

As SQL expert, could you review this PR?

In this PR QueryEntity.java was changed. And it may impact whole SQL engine.

--
Alexey Kuznetsov
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Dmitriy Pavlov
Hi Vladimir,

When you could take a look?

Sincerely,
Dmitriy Pavlov

пт, 23 мар. 2018 г. в 5:50, Alexey Kuznetsov <[hidden email]>:

> Vladimir,
>
> As SQL expert, could you review this PR?
>
> In this PR QueryEntity.java was changed. And it may impact whole SQL
> engine.
>
> --
> Alexey Kuznetsov
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Nikolay Izhikov-2
In reply to this post by Alexey Kuznetsov
I will take a look at the next week.

В Пт, 23/03/2018 в 09:50 +0700, Alexey Kuznetsov пишет:
> Vladimir,
>
> As SQL expert, could you review this PR?
>
> In this PR QueryEntity.java was changed. And it may impact whole SQL engine.
>

signature.asc (465 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Amir Akhmedov
Hi Nikolay,

Just a gentle reminder :)

Thanks,
Amir

On Fri, Mar 30, 2018 at 12:29 PM, Nikolay Izhikov <[hidden email]>
wrote:

> I will take a look at the next week.
>
> В Пт, 23/03/2018 в 09:50 +0700, Alexey Kuznetsov пишет:
> > Vladimir,
> >
> > As SQL expert, could you review this PR?
> >
> > In this PR QueryEntity.java was changed. And it may impact whole SQL
> engine.
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Nikolay Izhikov-2
Hello, Amir.

Please, see my jira comment - https://issues.apache.org/jira/browse/IGNITE-3999?focusedCommentId=16426473&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16426473

В Ср, 04/04/2018 в 20:16 -0400, Amir Akhmedov пишет:

> Hi Nikolay,
>
> Just a gentle reminder :)
>
> Thanks,
> Amir
>
> On Fri, Mar 30, 2018 at 12:29 PM, Nikolay Izhikov <[hidden email]>
> wrote:
>
> > I will take a look at the next week.
> >
> > В Пт, 23/03/2018 в 09:50 +0700, Alexey Kuznetsov пишет:
> > > Vladimir,
> > >
> > > As SQL expert, could you review this PR?
> > >
> > > In this PR QueryEntity.java was changed. And it may impact whole SQL
> >
> > engine.
> > >

signature.asc (465 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Amir Akhmedov
Hi Nikolay,
I fixed the comments. Please check them and let me know.

Thanks,
Amir

On Thu, Apr 5, 2018 at 12:30 AM, Nikolay Izhikov <[hidden email]>
wrote:

> Hello, Amir.
>
> Please, see my jira comment - https://issues.apache.org/
> jira/browse/IGNITE-3999?focusedCommentId=16426473&page=com.atlassian.jira.
> plugin.system.issuetabpanels:comment-tabpanel#comment-16426473
>
> В Ср, 04/04/2018 в 20:16 -0400, Amir Akhmedov пишет:
> > Hi Nikolay,
> >
> > Just a gentle reminder :)
> >
> > Thanks,
> > Amir
> >
> > On Fri, Mar 30, 2018 at 12:29 PM, Nikolay Izhikov <[hidden email]>
> > wrote:
> >
> > > I will take a look at the next week.
> > >
> > > В Пт, 23/03/2018 в 09:50 +0700, Alexey Kuznetsov пишет:
> > > > Vladimir,
> > > >
> > > > As SQL expert, could you review this PR?
> > > >
> > > > In this PR QueryEntity.java was changed. And it may impact whole SQL
> > >
> > > engine.
> > > >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Amir Akhmedov
Hi Nikolay,

Did you have a chance to check my changes?

Thanks,
Amir
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Nikolay Izhikov-2
Hello, Amir.

Sorry, but no.

I will take a look in a next few days.


В Ср, 11/04/2018 в 14:51 +0000, Amir Akhmedov пишет:
> Hi Nikolay,
>
> Did you have a chance to check my changes?
>
> Thanks,
> Amir

signature.asc (465 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Amir Akhmedov
Hi Nikolay, Dmitry Pavlov,
Can you take a look at this PR or maybe you have an idea when it probably
can be reviewed?

Thanks,
Amir

On Wed, Apr 11, 2018 at 10:57 AM, Nikolay Izhikov <[hidden email]>
wrote:

> Hello, Amir.
>
> Sorry, but no.
>
> I will take a look in a next few days.
>
>
> В Ср, 11/04/2018 в 14:51 +0000, Amir Akhmedov пишет:
> > Hi Nikolay,
> >
> > Did you have a chance to check my changes?
> >
> > Thanks,
> > Amir
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Vladimir Ozerov
Hi,

I provided my comments in the ticket.

On Tue, May 8, 2018 at 5:54 AM, Amir Akhmedov <[hidden email]>
wrote:

> Hi Nikolay, Dmitry Pavlov,
> Can you take a look at this PR or maybe you have an idea when it probably
> can be reviewed?
>
> Thanks,
> Amir
>
> On Wed, Apr 11, 2018 at 10:57 AM, Nikolay Izhikov <[hidden email]>
> wrote:
>
> > Hello, Amir.
> >
> > Sorry, but no.
> >
> > I will take a look in a next few days.
> >
> >
> > В Ср, 11/04/2018 в 14:51 +0000, Amir Akhmedov пишет:
> > > Hi Nikolay,
> > >
> > > Did you have a chance to check my changes?
> > >
> > > Thanks,
> > > Amir
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Dmitriy Pavlov
Hi Vladimir, thank you for stepping in.

This case confirms the usefulness of the practice of discussing the problem
at the devlist before implementation.

вт, 8 мая 2018 г. в 10:12, Vladimir Ozerov <[hidden email]>:

> Hi,
>
> I provided my comments in the ticket.
>
> On Tue, May 8, 2018 at 5:54 AM, Amir Akhmedov <[hidden email]>
> wrote:
>
> > Hi Nikolay, Dmitry Pavlov,
> > Can you take a look at this PR or maybe you have an idea when it probably
> > can be reviewed?
> >
> > Thanks,
> > Amir
> >
> > On Wed, Apr 11, 2018 at 10:57 AM, Nikolay Izhikov <[hidden email]>
> > wrote:
> >
> > > Hello, Amir.
> > >
> > > Sorry, but no.
> > >
> > > I will take a look in a next few days.
> > >
> > >
> > > В Ср, 11/04/2018 в 14:51 +0000, Amir Akhmedov пишет:
> > > > Hi Nikolay,
> > > >
> > > > Did you have a chance to check my changes?
> > > >
> > > > Thanks,
> > > > Amir
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

dsetrakyan
Igniters, let's make sure we add ticket description to the email subject in
future, so the community will know what the ticket is about. Not everyone
has time to look for the ticket, especially when the link is not even
provided in the email.

For example, the subject of this thread should have been: "IGNITE-3999:
Support case insensitive search in SQL"

D.

On Tue, May 8, 2018 at 2:28 PM, Dmitry Pavlov <[hidden email]> wrote:

> Hi Vladimir, thank you for stepping in.
>
> This case confirms the usefulness of the practice of discussing the problem
> at the devlist before implementation.
>
> вт, 8 мая 2018 г. в 10:12, Vladimir Ozerov <[hidden email]>:
>
> > Hi,
> >
> > I provided my comments in the ticket.
> >
> > On Tue, May 8, 2018 at 5:54 AM, Amir Akhmedov <[hidden email]>
> > wrote:
> >
> > > Hi Nikolay, Dmitry Pavlov,
> > > Can you take a look at this PR or maybe you have an idea when it
> probably
> > > can be reviewed?
> > >
> > > Thanks,
> > > Amir
> > >
> > > On Wed, Apr 11, 2018 at 10:57 AM, Nikolay Izhikov <[hidden email]
> >
> > > wrote:
> > >
> > > > Hello, Amir.
> > > >
> > > > Sorry, but no.
> > > >
> > > > I will take a look in a next few days.
> > > >
> > > >
> > > > В Ср, 11/04/2018 в 14:51 +0000, Amir Akhmedov пишет:
> > > > > Hi Nikolay,
> > > > >
> > > > > Did you have a chance to check my changes?
> > > > >
> > > > > Thanks,
> > > > > Amir
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Amir Akhmedov
Hi All,
It's definitely my bad, I had to synch up with community on ticket's
relevance.
Would be nice for future contributors to mention "do get updates on
ticket's relevance for outdated tickets" in Ignite's "How to contribute"
webpage.

Thanks,
Amir

On Tue, May 8, 2018 at 8:39 AM, Dmitriy Setrakyan <[hidden email]>
wrote:

> Igniters, let's make sure we add ticket description to the email subject in
> future, so the community will know what the ticket is about. Not everyone
> has time to look for the ticket, especially when the link is not even
> provided in the email.
>
> For example, the subject of this thread should have been: "IGNITE-3999:
> Support case insensitive search in SQL"
>
> D.
>
> On Tue, May 8, 2018 at 2:28 PM, Dmitry Pavlov <[hidden email]>
> wrote:
>
> > Hi Vladimir, thank you for stepping in.
> >
> > This case confirms the usefulness of the practice of discussing the
> problem
> > at the devlist before implementation.
> >
> > вт, 8 мая 2018 г. в 10:12, Vladimir Ozerov <[hidden email]>:
> >
> > > Hi,
> > >
> > > I provided my comments in the ticket.
> > >
> > > On Tue, May 8, 2018 at 5:54 AM, Amir Akhmedov <[hidden email]
> >
> > > wrote:
> > >
> > > > Hi Nikolay, Dmitry Pavlov,
> > > > Can you take a look at this PR or maybe you have an idea when it
> > probably
> > > > can be reviewed?
> > > >
> > > > Thanks,
> > > > Amir
> > > >
> > > > On Wed, Apr 11, 2018 at 10:57 AM, Nikolay Izhikov <
> [hidden email]
> > >
> > > > wrote:
> > > >
> > > > > Hello, Amir.
> > > > >
> > > > > Sorry, but no.
> > > > >
> > > > > I will take a look in a next few days.
> > > > >
> > > > >
> > > > > В Ср, 11/04/2018 в 14:51 +0000, Amir Akhmedov пишет:
> > > > > > Hi Nikolay,
> > > > > >
> > > > > > Did you have a chance to check my changes?
> > > > > >
> > > > > > Thanks,
> > > > > > Amir
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: IGNITE-3999 review

Dmitriy Pavlov
Hi Amir,

I've updated How To Contribute (
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute )
section 'beginning work':

(OPTIONAL) It is recommended to clarify the relevance and correctness of
the ticket in the community before doing actual implementation. To do this,
you can write a letter to the dev.list <[hidden email]> and wait
for answer.

This article has one note in 'tickets' section related to dev list:
"... unassigned
and unscheduled tickets after discussion on project's dev list. " ,- but it
is not so obivious that it is good practice.

I've used optional because some of contributors can be quite sure ticket is
actual, so I think it is not required to mandate everyone to start
discussion.

Sincerely,
Dmitriy Pavlov

вт, 8 мая 2018 г. в 19:43, Amir Akhmedov <[hidden email]>:

> Hi All,
> It's definitely my bad, I had to synch up with community on ticket's
> relevance.
> Would be nice for future contributors to mention "do get updates on
> ticket's relevance for outdated tickets" in Ignite's "How to contribute"
> webpage.
>
> Thanks,
> Amir
>
> On Tue, May 8, 2018 at 8:39 AM, Dmitriy Setrakyan <[hidden email]>
> wrote:
>
> > Igniters, let's make sure we add ticket description to the email subject
> in
> > future, so the community will know what the ticket is about. Not everyone
> > has time to look for the ticket, especially when the link is not even
> > provided in the email.
> >
> > For example, the subject of this thread should have been: "IGNITE-3999:
> > Support case insensitive search in SQL"
> >
> > D.
> >
> > On Tue, May 8, 2018 at 2:28 PM, Dmitry Pavlov <[hidden email]>
> > wrote:
> >
> > > Hi Vladimir, thank you for stepping in.
> > >
> > > This case confirms the usefulness of the practice of discussing the
> > problem
> > > at the devlist before implementation.
> > >
> > > вт, 8 мая 2018 г. в 10:12, Vladimir Ozerov <[hidden email]>:
> > >
> > > > Hi,
> > > >
> > > > I provided my comments in the ticket.
> > > >
> > > > On Tue, May 8, 2018 at 5:54 AM, Amir Akhmedov <
> [hidden email]
> > >
> > > > wrote:
> > > >
> > > > > Hi Nikolay, Dmitry Pavlov,
> > > > > Can you take a look at this PR or maybe you have an idea when it
> > > probably
> > > > > can be reviewed?
> > > > >
> > > > > Thanks,
> > > > > Amir
> > > > >
> > > > > On Wed, Apr 11, 2018 at 10:57 AM, Nikolay Izhikov <
> > [hidden email]
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hello, Amir.
> > > > > >
> > > > > > Sorry, but no.
> > > > > >
> > > > > > I will take a look in a next few days.
> > > > > >
> > > > > >
> > > > > > В Ср, 11/04/2018 в 14:51 +0000, Amir Akhmedov пишет:
> > > > > > > Hi Nikolay,
> > > > > > >
> > > > > > > Did you have a chance to check my changes?
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Amir
> > > > > >
> > > > >
> > > >
> > >
> >
>