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
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
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
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
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
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
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
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
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
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
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.
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
> 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
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
чт, 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
>> . 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
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.
>
>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
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.
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
;> 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
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.
>
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
> >>
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
erfaces and desired
> usages.
> > >>>>
> > >>>> This will help to the community and me personally better understand
> your idea and share feedback.
> > >>>>
> > >>>> Thanks.
> > >>>>
> > >>&
derstand your
> >>>> idea and share feedback.
> >>>>
> >>>> Thanks.
> >>>>
> >>>>> 23 янв. 2020 г., в 17:15, Andrey Gura написал(а):
> >>>>>
> >>>>> Nikolay,
> >
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).
>>>&
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
>>
>>> 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
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 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
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
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
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.
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
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
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]
> 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.
>
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.
>
>
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
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
> 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?
>> 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
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
+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.
>
>
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
+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
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
> 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
>> 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
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,
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
>> 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
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
>
> 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
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
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
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
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
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
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
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?
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
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
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
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,
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
>> написал(а):
>>
>>
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
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, Николай
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.
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,
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
72 matches
Mail list logo