Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate Aggregator.getName and AggregatorFactory.getAggregatorStartValue. #3572

Merged
merged 1 commit into from Oct 31, 2016

Conversation

gianm
Copy link
Contributor

@gianm gianm commented Oct 15, 2016

Partial rework of #3387, plus deprecating AggregatorFactory.getAggregatorStartValue too. Neither was really used for much. This deprecates rather than removes the functions, to prevent errors for people that have implemented custom aggregators.

From the original PR, this can also help quite a bit with memory use of OnheapIncrementalIndex when there are a large number of aggregators. #3387 (comment)

@gianm gianm added this to the 0.9.3 milestone Oct 15, 2016
@gianm gianm force-pushed the deprecate-unused-agg-stuff branch 2 times, most recently from e44d06c to 0ff1988 Compare October 15, 2016 04:18
* Returns the starting value for a corresponding aggregator. For example, 0 for sums, - Infinity for max, an empty mogrifier
*
* @return the starting value for a corresponding aggregator.
* Deprecated, to be removed in 0.10.0.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add something like "see /pull/3572 for details"?

@@ -37,7 +37,7 @@
void reset();
Object get();
float getFloat();
String getName();
@Deprecated String getName();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add something like "see /pull/3572 for details"?

Copy link
Member

@nishantmonu51 nishantmonu51 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍, Lets also open an issue for 0.10.0 to remove deprecated methods.

@gianm
Copy link
Contributor Author

gianm commented Oct 18, 2016

@leventov @nishantmonu51 I raised #3588 and mentioned it in the javadocs for the deprecated methods.

@gianm gianm force-pushed the deprecate-unused-agg-stuff branch 3 times, most recently from 43365a8 to 445bf09 Compare October 31, 2016 17:49
@gianm
Copy link
Contributor Author

gianm commented Oct 31, 2016

rebased against master, although tests cannot pass until #3624 is merged.

@gianm gianm reopened this Oct 31, 2016
@gianm
Copy link
Contributor Author

gianm commented Oct 31, 2016

rebased against master again.

@fjy
Copy link
Contributor

fjy commented Oct 31, 2016

👍

@fjy fjy merged commit 89d9c61 into apache:master Oct 31, 2016
@gianm gianm deleted the deprecate-unused-agg-stuff branch October 31, 2016 22:38
fundead pushed a commit to fundead/druid that referenced this pull request Dec 7, 2016
dgolitsyn pushed a commit to metamx/druid that referenced this pull request Feb 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants