Hi Atri, Looks good to me, I've merged the changes. Please resolve the ticket.
-Val On Wed, Mar 17, 2021 at 5:07 AM Atri Sharma <a...@apache.org> wrote: > Hi Valentine, > > Hoping you are well. > > Please let me know if the PR looks ok. > > Regards, > > Atri > > > On Thu, 11 Mar 2021, 12:09 Atri Sharma, <a...@apache.org> wrote: > > > Hi Val, > > > > Thanks for taking a look. I have updated the PR, please see and let me > > know your thoughts and feedback. > > > > Regards, > > > > Atri > > > > On Thu, Mar 11, 2021 at 6:16 AM Valentin Kulichenko > > <valentin.kuliche...@gmail.com> wrote: > > > > > > Atri, > > > > > > I've added my comments to the PR. > > > > > > -Val > > > > > > On Wed, Mar 10, 2021 at 2:44 AM Atri Sharma <a...@apache.org> wrote: > > > > > > > I have just updated the PR: > > > > > > > > https://github.com/apache/ignite/pull/8820 > > > > > > > > Please review. > > > > > > > > On Wed, Mar 10, 2021 at 1:51 PM Atri Sharma <a...@apache.org> wrote: > > > > > > > > > > Hi Val, > > > > > > > > > > Thank you for taking the time on this one. > > > > > > > > > > The main reason as to why I chose that signature was because I felt > > it > > > > > gave a clear idea of the interface that the user should expect when > > > > > using the method. So essentially, the user is providing a callback > > > > > listener himself/herself and the API is just using that to tie the > > > > > lifecycle of holding the semaphore. > > > > > > > > > > However, I do see your point and feel that the current signature > that > > > > > the PR has will limit the usability of the method and make users > jump > > > > > through more hoops. I have changed the signature to accept Callable > > > > > returning T. > > > > > > > > > > Regards, > > > > > > > > > > Atri > > > > > > > > > > On Wed, Mar 10, 2021 at 5:29 AM Valentin Kulichenko > > > > > <valentin.kuliche...@gmail.com> wrote: > > > > > > > > > > > > Hi Atri, > > > > > > > > > > > > First and foremost, we need to clarify the API for this > > functionality. > > > > > > There are two options presented in the ticket, but no clear > > decision > > > > about > > > > > > which one should be used. I personally think that the original > > > > signature > > > > > > (the one in the ticket description) makes more sense, but it > looks > > like > > > > > > you've implemented an alternative suggested in the comments. What > > was > > > > your > > > > > > motivation behind that? > > > > > > > > > > > > As for where the method can be located, I'm OK if we add it > > directly > > > > to the > > > > > > IgniteSemaphore interface. > > > > > > > > > > > > -Val > > > > > > > > > > > > On Tue, Mar 9, 2021 at 8:22 AM Atri Sharma <a...@apache.org> > > wrote: > > > > > > > > > > > > > Gentle ping > > > > > > > > > > > > > > On Mon, 8 Mar 2021, 13:51 Atri Sharma, <a...@apache.org> > wrote: > > > > > > > > > > > > > > > Hi All, > > > > > > > > > > > > > > > > I would like to start a discussion around IGNITE-2399. > > > > > > > > > > > > > > > > Background: The typical use of IgniteSemaphore consists of > > > > acquiring > > > > > > > > the semaphore, performing the task and releasing the > semaphore. > > > > This > > > > > > > > JIRA proposes a new method which will accept a callable, > > acquire > > > > the > > > > > > > > semaphore, and return a future. Upon completion of the > future, > > the > > > > > > > > semaphore is released. > > > > > > > > > > > > > > > > This API seems useful for an easy encapsulation of the > > described > > > > use > > > > > > > > case and does not cause an internal flux since all the > changes > > are > > > > > > > > focused at the public API. > > > > > > > > > > > > > > > > WIP PR for the same: > > > > > > > > > > > > > > > > https://github.com/apache/ignite/pull/8820 > > > > > > > > > > > > > > > > Please share your thoughts and comments. > > > > > > > > > > > > > > > > -- > > > > > > > > Regards, > > > > > > > > > > > > > > > > Atri > > > > > > > > Apache Concerted > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Regards, > > > > > > > > > > Atri > > > > > Apache Concerted > > > > > > > > > > > > > > > > -- > > > > Regards, > > > > > > > > Atri > > > > Apache Concerted > > > > > > > > -- > > Regards, > > > > Atri > > Apache Concerted > > >
