Re: Internal classes are exposed in public API

2020-02-06 Thread Alexey Goncharuk
The discussion thread is started: http://apache-ignite-developers.2346864.n4.nabble.com/DISCUSS-Public-API-deprecation-rules-td45647.html ср, 5 февр. 2020 г. в 12:43, Alexey Goncharuk : > Sorry for the rush, I think I got it - it's right in the voting process > [1]. This would be a vote on a

Re: Internal classes are exposed in public API

2020-02-05 Thread Alexey Goncharuk
Sorry for the rush, I think I got it - it's right in the voting process [1]. This would be a vote on a procedural issue of how to handle code deprecation. I'll start the discussion soon, let's agree on the alternatives and then start the vote. [1] https://www.apache.org/foundation/voting.html

Re: Internal classes are exposed in public API

2020-02-05 Thread Alexey Goncharuk
Denis, I just realized that this vote would not be a regular Apache vote we used to run. Usually we vote for a change to be accepted or rejected. Here, on the other hand, we need to choose between two alternatives, and I am not sure yet how this should be formally handled. Dmitriy, perhaps, you

Re: Internal classes are exposed in public API

2020-02-04 Thread Denis Magda
Let's run a vote if that's the only option to come to a consensus. It will be best if either Alex Goncharuk, Andrey Gura or Nikolay Izhikov creates a discussion thread stating the problem and possible choices. Folks, who would like to take over? Generally, the vote should help us to decide how to

Re: Internal classes are exposed in public API

2020-02-04 Thread Maxim Muzafarov
Folks, Let's start a vote? On Mon, 3 Feb 2020 at 15:05, Andrey Gura wrote: > > Just post here article from Oracle documentation "How and When To > Deprecate APIs" [1]. > > [1] > https://docs.oracle.com/javase/8/docs/technotes/guides/javadoc/deprecation/deprecation.html > > On Fri, Jan 31, 2020

Re: Internal classes are exposed in public API

2020-02-03 Thread Andrey Gura
Just post here article from Oracle documentation "How and When To Deprecate APIs" [1]. [1] https://docs.oracle.com/javase/8/docs/technotes/guides/javadoc/deprecation/deprecation.html On Fri, Jan 31, 2020 at 10:44 AM Pavel Tupitsyn wrote: > > Agree with Andrey, let's remove deprecation for now

Re: Internal classes are exposed in public API

2020-01-30 Thread Pavel Tupitsyn
Agree with Andrey, let's remove deprecation for now and unblock the release. On Thu, Jan 30, 2020 at 10:23 PM Andrey Gura wrote: > I'll just repeat one thought with some changes. > > There are at least two groups of people in this discussion. One group > sure that new API is complete and

Re: Internal classes are exposed in public API

2020-01-30 Thread Andrey Gura
I'll just repeat one thought with some changes. There are at least two groups of people in this discussion. One group sure that new API is complete and production ready while other group disagree with it. Obviously we can't reach consensus about it right now. But we can do it in the future. At

Re: Internal classes are exposed in public API

2020-01-30 Thread Nikolay Izhikov
Alexey. I answered to your examples and issues you provide. But, it seems the discussion of the API and the Java code itself is not the goal of this thread anymore. > Should we provide a way to know the number of metrics and registries in > advance? No. If you think this is the real use-as

Re: Internal classes are exposed in public API

2020-01-30 Thread Denis Magda
Folks, seriously, we should deprecate an existing API only after the new one is no longer considered experimental. There might be API and configuration changes in the experimental API before it's announced as GA. I would encourage us to do this properly - let's release the new APIs labeling them

Re: Internal classes are exposed in public API

2020-01-30 Thread Alexey Goncharuk
Nikita, Disagree here. I already gave an example in this thread of how you need to peek into configuration in order to obtain an instance of exporter SPI which may not necessarily be the type you need. That's why IGNITE-12553 was created in the first place.

Re: Internal classes are exposed in public API

2020-01-30 Thread Nikita Amelchev
Folks, I have some experience using the new metrics API. The new format is easy to use from the user point and approved by the community. It provides a single metric format and one entry point. I believe that they are able to replace existing ones. I don't see any issues with the new metrics. The

Re: Internal classes are exposed in public API

2020-01-30 Thread Andrey Gura
> We doesn’t come to an agreement that API should be changed. > We should discuss the design of the Metric API and your proposals for it in > another thread. > Please, avoid arguments like «this API will be 100% changed» in this > discussion. I just explain why this item is acceptable to me. I

Re: Internal classes are exposed in public API

2020-01-30 Thread Andrey Mashenkov
Hi Igniters, Let me share some thoughts. Abstractly. 1. @Experimental API. When user see API marked as experimental usually it is a red flag for using such API in prod and also he can't count the API stability: - API itself if not stable and one can have compilation issues with next versions at

Re: Internal classes are exposed in public API

2020-01-30 Thread Alexey Goncharuk
чт, 30 янв. 2020 г. в 15:18, Anton Vinogradov : > We should have all the options. > > Deprecated -> will be removed at major release (because this way is slow or > overcomplicated) but still be used now, > Not only does it mean that the API should be removed in a major release, but there is a

Re: Internal classes are exposed in public API

2020-01-30 Thread Anton Vinogradov
>> . Remove @Deprecated from old API (because it strange to have one >> deprecated API and second experimental API) >> 2. Add @IgniteExperimetnal to new API (because... see item. 1) -1 We should have all the options. Deprecated -> will be removed at major release (because this way is slow or

Re: Internal classes are exposed in public API

2020-01-30 Thread Nikolay Izhikov
Hello. > 2. Add @IgniteExperimetnal to new API (because... see item. 1 +1 > 1. Remove @Deprecated from old API (because it strange to have one deprecated > API and second experimental API) -1 I propose to update deprecation message and provide metric name for each deprecated method. >

Re: Internal classes are exposed in public API

2020-01-30 Thread Andrey Gura
>From my point of view we still don't have consensus. I think we should do at least the following: 1. Remove @Deprecated from old API (because it strange to have one deprecated API and second experimental API) 2. Add @IgniteExperimetnal to new API (because... see item. 1) 3. Do not merge

Re: Internal classes are exposed in public API

2020-01-30 Thread Alexey Goncharuk
Folks, I tried to re-read the whole thread and honestly got lost at the end :) Do we have a consensus (if yes, what are the steps?) or should we have a call as Maxim suggested? I think it is in our best interest to get this agreed upon fast to release AI 2.8 soon.

Re: Internal classes are exposed in public API

2020-01-28 Thread Nikolay Izhikov
ts and suggestions. >>>>>>>>> >>>>>>>>>> I think we should start a discussion from the simplified >>>>> explanation of: >>>>>>>>>> 1. API intentions - What we want to gain with it? What is our focu

Re: Internal classes are exposed in public API

2020-01-28 Thread Maxim Muzafarov
;> http://apache-ignite-developers.2346864.n4.nabble.com/IEP-35-Metrics-management-in-Ignite-tp43243.html > > >>>>>> > > >>>>>> On Thu, Jan 23, 2020 at 5:42 PM Николай Ижиков > > >> wrote: > > >>>>>>> > &g

Re: Internal classes are exposed in public API

2020-01-27 Thread Andrey Gura
changes > >>>>>>> > >>>>>>> We don’t discuss your changes and your proposals for the Metric API. > >>>>>>> So I don’t think we can make the existence of some PR is an > >> argument to introduce(or not introduce) this facade. >

Re: Internal classes are exposed in public API

2020-01-27 Thread Maxim Muzafarov
h > >> my changes > >>>>>>> > >>>>>>> We don’t discuss your changes and your proposals for the Metric API. > >>>>>>> So I don’t think we can make the existence of some PR is an > >>

Re: Internal classes are exposed in public API

2020-01-27 Thread Nikolay Izhikov
gt;>>>> 1. Review `IgniteMetric` facade and introduce it to the users as an >> experimental API. >>>>>>> >>>>>>> 2. Discuss your proposal to the Metric API in the separate thread >> you are referencing. >>>>>&g

Re: Internal classes are exposed in public API

2020-01-27 Thread Alexey Goncharuk
erfaces and desired > usages. > > >>>> > > >>>> This will help to the community and me personally better understand > your idea and share feedback. > > >>>> > > >>>> Thanks. > > >>>> > > >>&

Re: Internal classes are exposed in public API

2020-01-27 Thread Maxim Muzafarov
derstand your > >>>> idea and share feedback. > >>>> > >>>> Thanks. > >>>> > >>>>> 23 янв. 2020 г., в 17:15, Andrey Gura написал(а): > >>>>> > >>>>> Nikolay, > >

Re: Internal classes are exposed in public API

2020-01-25 Thread Николай Ижиков
nged during >>>>> IGNITE-11927 implementation so your changes are incompatible with my >>>>> changes. >>>>> >>>>> ReadOnlyMetricRegistry will be removed at all (still exists in a >>>>> couple of places where it uses). >>>&

Re: Internal classes are exposed in public API

2020-01-24 Thread Andrey Gura
ncompatible with my > >>> changes. > >>> > >>> ReadOnlyMetricRegistry will be removed at all (still exists in a > >>> couple of places where it uses). > >>> > >>> Also I don't want to introduce IgniteMetric facade in this rush

Re: Internal classes are exposed in public API

2020-01-23 Thread Николай Ижиков
>> >>> I agree with Denis. We should mark current API as experimental and >>> continue IEP-35 development. See my process proposal [1] described >>> early this thread. We can release Apache Ignite 2.8.1 specially for >>> this changes. >>> Publ

Re: Internal classes are exposed in public API

2020-01-23 Thread Andrey Gura
See my process proposal [1] described > > early this thread. We can release Apache Ignite 2.8.1 specially for > > this changes. > > Public APIs require deeper thinking in order to provide comprehensive, > > consistent and convenient way of metrics management for end users. &g

Re: Internal classes are exposed in public API

2020-01-23 Thread Николай Ижиков
re deeper thinking in order to provide comprehensive, > consistent and convenient way of metrics management for end users. > > Let's add IgniteExperimental, release 2.8 and finish metrics related > issues after it. > > [1] > http://apache-ignite-developers.2346864.n4.nabble.com/Int

Re: Internal classes are exposed in public API

2020-01-23 Thread Andrey Gura
after it. [1] http://apache-ignite-developers.2346864.n4.nabble.com/Internal-classes-are-exposed-in-public-API-tp45146p45185.html On Wed, Jan 22, 2020 at 5:21 PM Николай Ижиков wrote: > > Hello, Igniters. > > * IGNITE-12552: Move ReadOnlyMetricRegistry to public API merged to t

Re: Internal classes are exposed in public API

2020-01-22 Thread Николай Ижиков
Hello, Igniters. * IGNITE-12552: Move ReadOnlyMetricRegistry to public API merged to the master and cherry-picked to the 2.8. So the main issue with the Metric API solved. * I raised the PR [1] to fix second issue with the new Metric API: absence of the public Java API to get metrics. This PR

Re: Internal classes are exposed in public API

2020-01-21 Thread Николай Ижиков
Hello, Igniters. Alexey approved my PR [1] regarding fixing public API for metric exporters. I’m waiting for a bot visa and merge this PR. As we discussed, the metrics API will be marked with IgniteExperimental annotation. If anyone has any objection to this plan, please provide your feedback.

Re: Internal classes are exposed in public API

2020-01-21 Thread Николай Ижиков
Thanks, for the review Alexey. I will fix your comment and try to implement Monitoring facade, shortly. > 21 янв. 2020 г., в 13:32, Alexey Goncharuk > написал(а): > > Nikolay, > > I left a single comment in the PR about the histogram metric. I think the > API looks much cleaner now. > > I

Re: Internal classes are exposed in public API

2020-01-21 Thread Alexey Goncharuk
Nikolay, I left a single comment in the PR about the histogram metric. I think the API looks much cleaner now. I will take care of the @IgniteExperimental annotation. пн, 20 янв. 2020 г. в 20:55, Николай Ижиков : > Alexey. > > PR [1] is waiting for your review. > Please, take a look. > > I

Re: Internal classes are exposed in public API

2020-01-20 Thread Николай Ижиков
Alexey. PR [1] is waiting for your review. Please, take a look. I think we should do the following before 2.8 release * Introduce new @IgniteExperimental annotation as discussed. * Mark Monitoring API with it. * merge «[IEP-35] Expose MetricRegistry to the public API» to 2.8 * merge «[IEP-35]

Re: Internal classes are exposed in public API

2020-01-20 Thread Andrey Gura
> Let’s move from the long letters to the code. Let's thinking before coding. Development is not only coding. Why such a rush? Measure thrice and cut once. "Long letters" is way of discussion before writing code. And it's ok. On Mon, Jan 20, 2020 at 1:32 PM Николай Ижиков wrote: > > Andrey. >

Re: Internal classes are exposed in public API

2020-01-20 Thread Alexey Goncharuk
Nikolay, Should we wait for both of the tickets given that we agreed that we are putting an experimental marker on the new APIs? I'm ok to fix only the first one and add the experimental marker so that we do not delay 2.8 release. пн, 20 янв. 2020 г. в 13:32, Николай Ижиков : > Andrey. > >

Re: Internal classes are exposed in public API

2020-01-20 Thread Николай Ижиков
Andrey. Let’s move from the long letters to the code. If you want to change API - please, propose this changes. I think everybody wins if we make our API better. Please, describe proposed changes. It would be great if you have some examples or PR for it. As for this release, I have plans to

Re: Internal classes are exposed in public API

2020-01-20 Thread Andrey Gura
It solves problem. On Mon, Jan 20, 2020 at 12:09 PM Alexey Goncharuk wrote: > > After giving it some thought, I agree with Denis - there is nothing wrong > with exposing the new APIs, we just need to make it clear that we are still > going to change it. > > Should we Introduce something like

Re: Internal classes are exposed in public API

2020-01-20 Thread Andrey Gura
> However, we should do this only when the new APIs are production-ready. Denis, the problem is in understanding what does it mean "production ready". Obviously we have different understanding of this term. > What if release the improvements labeling as EA instead of hiding them completely?

Re: Internal classes are exposed in public API

2020-01-20 Thread Andrey Gura
>> Also I agree with Alexey about introducing public IgniteMonitoring facade > This is not an issue with the API. > Just the new feature that doesn’t affects API. I disagree. It's part of API and it affects user experience. > Moreover, I create a ticket to fix it, already. Creating an issue

Re: Internal classes are exposed in public API

2020-01-20 Thread Вячеслав Коптилин
Hello, I totally agree. It would be nice to have a marker that can be used to indicate that the feature may be changed in future releases and should be used to your own risk. Thanks, S. пн, 20 янв. 2020 г. в 12:16, Anton Vinogradov : > +1 to @IgniteExperimental > > On Mon, Jan 20, 2020 at

Re: Internal classes are exposed in public API

2020-01-20 Thread Anton Vinogradov
+1 to @IgniteExperimental On Mon, Jan 20, 2020 at 12:09 PM Alexey Goncharuk < alexey.goncha...@gmail.com> wrote: > After giving it some thought, I agree with Denis - there is nothing wrong > with exposing the new APIs, we just need to make it clear that we are still > going to change it. > >

Re: Internal classes are exposed in public API

2020-01-20 Thread Alexey Goncharuk
After giving it some thought, I agree with Denis - there is nothing wrong with exposing the new APIs, we just need to make it clear that we are still going to change it. Should we Introduce something like @IgniteExperimental annotation (I believe it has been discussed a dozen of times on the

Re: Internal classes are exposed in public API

2020-01-17 Thread Nikolay Izhikov
+1 to mark feature or whole release as EA. пт, 17 янв. 2020 г., 23:00 Denis Magda : > Folks, if you don't mind I'll share some thoughts/suggestions as an > observer who was not involved in the feature development. > > It's absolutely 'ok' to deprecate an API that is replaced with a much > better

Re: Internal classes are exposed in public API

2020-01-17 Thread Denis Magda
Folks, if you don't mind I'll share some thoughts/suggestions as an observer who was not involved in the feature development. It's absolutely 'ok' to deprecate an API that is replaced with a much better version. However, we should do this only when the new APIs are production-ready. If there are

Re: Internal classes are exposed in public API

2020-01-17 Thread Николай Ижиков
> Also I agree with Alexey about introducing public IgniteMonitoring facade This is not an issue with the API. Just the new feature that doesn’t affects API. Moreover, I create a ticket to fix it, already. > It's right. But if you add checking of statisticsEnabling property then test > will

Re: Internal classes are exposed in public API

2020-01-17 Thread Andrey Gura
>> All issues Alexey mentioned in starting letter are fixed with my PR [1]. >> I don’t think other issues you mentioned are blocker for the release. As I mentioned already IGNITE-11927 is part of IEP-35 and implementation seriously affects API's. Also I agree with Alexey about introducing public

Re: Internal classes are exposed in public API

2020-01-17 Thread Николай Ижиков
Andrey. All issues Alexey mentioned in starting letter are fixed with my PR [1]. I don’t think other issues you mentioned are blocker for the release. > I talk about ignored existing functionality. There is no existing tests that was broken by this contribution. If you know the issues with it,

Re: Internal classes are exposed in public API

2020-01-17 Thread Andrey Gura
The discussion is hot and can be endless. So in separate post I want propose my solution. 1. Moving IEP-35 API's to the internal package. 2. Create special feature branch B. 3. In branch B will be merged IGNITE-11927 4. Because IGNITE-11927 doesn't solve all problems we should propose solution

Re: Internal classes are exposed in public API

2020-01-17 Thread Andrey Gura
>> Because it is brand new API and it requires rewrite client code. > We doesn’t break backward compatibility. > The message is - this interface would be remove in the next major release. We don't know anything about development processes our users. I can admit that process could require that

Re: Internal classes are exposed in public API

2020-01-17 Thread Николай Ижиков
Andrey. > Because it is brand new API and it requires rewrite client code. We doesn’t break backward compatibility. The message is - this interface would be remove in the next major release. > ReadOnlyMetricRegistry > Form user stand point it is very strange interface which don't give me any >

Re: Internal classes are exposed in public API

2020-01-17 Thread Andrey Gura
> If I’m not missing something, you were one of the active reviewers of the > Metric API. Yes. But if I'm not missing some thing you were major developer of Metric API :) Shit happens. And it happened. >> The first, I agree with Alexey about deprecation of APIs that are still >> supported and

Re: Internal classes are exposed in public API

2020-01-17 Thread Николай Ижиков
Andrey, thanks for your opinion and your ownest critisism. I can’t wait for your contribution. If I’m not missing something, you were one of the active reviewers of the Metric API. > The first, I agree with Alexey about deprecation of APIs that are still > supported and don't offer reasonable

Re: Internal classes are exposed in public API

2020-01-17 Thread Andrey Gura
Hi, The first, I agree with Alexey about deprecation of APIs that are still supported and don't offer reasonable substitution. The second, from my point of view, we can't recommend MetricExporterSpi's because it are still not-production ready. There are some issues with it and usage of

Re: Internal classes are exposed in public API

2020-01-17 Thread Alexey Goncharuk
Nikolay, thanks! I am not insisting we must implement IGNITE-12553 right away nor am I saying it's mandatory. I just want to make sure we do not deprecate existing APIs unless we as a community are certain that they will be dropped and there is no need for the replacement. Will take a look at the

Re: Internal classes are exposed in public API

2020-01-17 Thread Николай Ижиков
Alex. OK, I may leverage your experience and create pure Java API. Ticket [1] created. But, personally, I don’t agree with you. Ignite has dozens of the API that theoretically have a usage scenario, but in real-world have 0 custom implementation and usages. Moreover, many APIs that were

Re: Internal classes are exposed in public API

2020-01-17 Thread Alexey Goncharuk
Nikolay, Why do you think this is a wrong usage pattern? From the top of my head, here is a few cases of direct metric API usage that I know are currently being used in production: * A custom task execution scheduling service with load balancing based on utilization metrics readings from Java

Re: Internal classes are exposed in public API

2020-01-16 Thread Николай Ижиков
Pavel, As I said before - I don’t see any real use-cases for your example. That’s why > I don’t create pure Java API for metric intentionally. What real-world use-case you are keeping in mind? Why do you want to do it in that way? The API you’re talking about is very simple and

Re: Internal classes are exposed in public API

2020-01-16 Thread Николай Ижиков
Hello, Pavel. > there should be an obvious public API to retrieve metrics What do you mean by «retrieve»? I don’t create pure Java API for metric intentionally. We have MetricExporterSPI for this purpose. It very simple from my point of view. What use cases for `Ignite.monitoring()` API exists?

Re: Internal classes are exposed in public API

2020-01-16 Thread Pavel Tupitsyn
Agree with Alexey, there should be an obvious public API to retrieve metrics On Thu, Jan 16, 2020 at 8:23 PM Alexey Goncharuk wrote: > That's an option, I agree. Yet for me, from the usability point of view, it > would be quite strange - to get an instance of the monitoring interface, I > would

Re: Internal classes are exposed in public API

2020-01-16 Thread Alexey Goncharuk
That's an option, I agree. Yet for me, from the usability point of view, it would be quite strange - to get an instance of the monitoring interface, I would need to do JmxMetricExporterSpi metrics = (JmxMetricExporterSpi)ignite.configuration().getMetricExporterSpi()[0]; which is too verbose and

Re: Internal classes are exposed in public API

2020-01-16 Thread Николай Ижиков
May be we should enable JMX exporter, by default? This would provide the same value for the user as `IgniteMonitoring` you propose. > 16 янв. 2020 г., в 20:06, Alexey Goncharuk > написал(а): > > I think it would make sense to make something like a new IgniteMonitoring > facade on Ignite

Re: Internal classes are exposed in public API

2020-01-16 Thread Alexey Goncharuk
I think it would make sense to make something like a new IgniteMonitoring facade on Ignite interface and add an ability to obtain a metric value through this facade, not through an exporter. This would be a straightforward replacement for all old metrics interfaces. чт, 16 янв. 2020 г. в 17:13,

Re: Internal classes are exposed in public API

2020-01-16 Thread Николай Ижиков
Ticket to export MetricRegistry to the public API created - https://issues.apache.org/jira/browse/IGNITE-12552 > 16 янв. 2020 г., в 16:58, Николай Ижиков написал(а): > > Do you propose to keep these interfaces forever? > >> 16 янв. 2020 г., в 16:52, Alexey Goncharuk >> написал(а): >> >>

Re: Internal classes are exposed in public API

2020-01-16 Thread Николай Ижиков
Do you propose to keep these interfaces forever? > 16 янв. 2020 г., в 16:52, Alexey Goncharuk > написал(а): > > Let's say I am upgrading from 2.7 to 2.8. Given that I figured out to > obtain an instance of the JMX exporter SPI, how should I map the old > 'interface + method name' to the new

Re: Internal classes are exposed in public API

2020-01-16 Thread Alexey Goncharuk
Let's say I am upgrading from 2.7 to 2.8. Given that I figured out to obtain an instance of the JMX exporter SPI, how should I map the old 'interface + method name' to the new metric name? I think deprecation of the public API may be a bit harsh in this case. чт, 16 янв. 2020 г. в 16:44, Николай

Re: Internal classes are exposed in public API

2020-01-16 Thread Николай Ижиков
Hello, Alexey. > * DataRegionMetric public interface is deprecated, however, the suggested > replacement class GridMetricsManager is internal and cannot be acquired by a > user. This makes impossible for the user to fix their code to not use > deprecated API May be it’s not clear text here.

Re: Internal classes are exposed in public API

2020-01-16 Thread Alexey Goncharuk
The ticket [1] was https://issues.apache.org/jira/browse/IGNITE-12550 чт, 16 янв. 2020 г. в 16:39, Alexey Goncharuk : > Igniters, Nikolay, > > I was adding a new metric in the scope of the ticket [1] and was surprised > by a few things: > * DataRegionMetric public interface is deprecated,

Internal classes are exposed in public API

2020-01-16 Thread Alexey Goncharuk
Igniters, Nikolay, I was adding a new metric in the scope of the ticket [1] and was surprised by a few things: * DataRegionMetric public interface is deprecated, however, the suggested replacement class GridMetricsManager is internal and cannot be acquired by a user. This makes impossible for