Skip to content

camel-influxdb2: deprecate retentionPolicy, which has no effect - #29709

Open
Dhairya-007 wants to merge 3 commits into
apache:mainfrom
Dhairya-007:fix/influxdb2-dead-retention-policy
Open

Dhairya-007 wants to merge 3 commits into
apache:mainfrom
Dhairya-007:fix/influxdb2-dead-retention-policy

Conversation

@Dhairya-007

Copy link
Copy Markdown
Contributor

Problem

camel-influxdb2 declares and documents two things that nothing reads:

  • the retentionPolicy endpoint option (InfluxDb2Endpoint, default default), and
  • the camelInfluxDB.RetentionPolicy header (InfluxDb2Constants.RETENTION_POLICY, carrying a @Metadata description).

Across the whole module, the only references to either are their own declaration, getter and setter. InfluxDb2Producer never asks for them.

A retention policy is an InfluxDB 1.x concept: its write API takes one per write, and camel-influxdb really does use it —

// camel-influxdb InfluxDbProducer
String retentionPolicy = exchange.getIn().getHeader(InfluxDbConstants.RETENTION_POLICY_HEADER, String.class);
...
connection.write(dataBaseName, retentionPolicy, p);

— while the InfluxDB 2.x write API this component calls has no such parameter. Retention is a property of the bucket.

The header even kept the 1.x spelling, camelInfluxDB.RetentionPolicy, while every other header in this component is CamelInfluxDB2… (CamelInfluxDB2MeasurementName, CamelInfluxDB2WritePrecision, …). That is what gives the copy away.

A user who sets either one gets no warning and no effect, and the option's default of default makes it look like a working setting.

Fix

Deprecate both with a deprecationNote, and say in the description that they are never read — the same treatment camel-aws2-s3vectors gave its unused SIMILARITY_THRESHOLD/SIMILARITY_SCORE headers and camel-tensorflow-serving gave its target header.

Changed:

  • InfluxDb2Endpoint and InfluxDb2Constants — @Deprecated + deprecationNote, and a description that says what is going on.
  • influxdb2.json and its camel-catalog copy — "deprecated": true plus the note, for both the parameter and the header. The two files are byte-identical afterwards.
  • InfluxDb2EndpointBuilderFactory — @Deprecated and the re-wrapped javadoc.
  • camel-4x-upgrade-guide-4_24.adoc — the entry a deprecation takes, pointing users at the bucket's retention period.

The generated files were updated by hand, since I cannot run maven here; I took the shape from the existing deprecated headers and re-wrapped the DSL javadoc at the generator's own width (69 characters of text at indent 9), checking first that the wrap reproduces the untouched lines. If the regen job still finds a difference I will take its patch.

Tests

No behaviour changes — the option and header did nothing before and do nothing now, they are only labelled honestly — so there is nothing new to assert. The existing camel-influxdb2 tests cover the writes, which this does not touch.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The @Deprecated annotation is correctly added to the private retentionPolicy field, but the public accessor methods getRetentionPolicy() and setRetentionPolicy() are not in this diff — meaning they remain unannotated.

In Java, @Deprecated on a private field does not propagate to its getters/setters. Any caller using endpoint.setRetentionPolicy(...) will receive no compiler deprecation warning, which defeats the purpose of the annotation.

Please add @Deprecated (and ideally a matching @deprecated Javadoc tag) to both accessor methods as well.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@Metadata(deprecationNote = "Never read by the component: the InfluxDB 2.x write API has no retention policy,"
+ " retention is a property of the bucket")
@Deprecated
private String retentionPolicy = "default";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Deprecated is placed on the private field, but the public accessors getRetentionPolicy() / setRetentionPolicy() are not in this diff and will remain unannotated. Java's @Deprecated does not propagate from a field to its getters/setters — callers won't see a compiler warning.

Please add @Deprecated (and a @deprecated Javadoc tag) to both methods.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Deprecated is placed on the private field, but the public accessors getRetentionPolicy() / setRetentionPolicy() are not in this diff and will remain unannotated. Java's @Deprecated does not propagate from a field to its getters/setters — callers won't see a compiler warning.

Please add @Deprecated (and a @deprecated Javadoc tag) to both methods.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@Metadata(deprecationNote = "Never read by the component: the InfluxDB 2.x write API has no retention policy,"
+ " retention is a property of the bucket")
@Deprecated
private String retentionPolicy = "default";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Deprecated is placed on the private field, but the public accessors getRetentionPolicy() / setRetentionPolicy() are not in this diff and will remain unannotated. Java's @Deprecated does not propagate from a field to its getters/setters — callers won't see a compiler warning.

Please add @Deprecated (and a @deprecated Javadoc tag) to both methods.

@github-actions

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TL;DR: the analysis is right, retentionPolicy and camelInfluxDB.RetentionPolicy are never read; one convention gap is left before this is ready.

I confirmed that in camel-influxdb2 the option and the header are only referenced by their own declaration and accessors; InfluxDb2Producer never reads them, so deprecating them with a deprecationNote and an upgrade-guide entry is the right treatment. The generated JSON follows the shape the schema generator emits ("... Deprecation note: ...").

One thing to add, the same point the bot raised: getRetentionPolicy() / setRetentionPolicy() in InfluxDb2Endpoint.java (lines 141 and 145) are not deprecated. The precedent you followed, apiKeyHeader in DoclingConfiguration (#29591), puts @Deprecated and a @deprecated Javadoc tag on both the getter and the setter, so Java callers get the compiler warning too:

/**
 * @deprecated never read: the InfluxDB 2.x write API has no retention policy, retention is a property of the bucket
 */
@Deprecated
public String getRetentionPolicy() {

(same for the setter). Heads-up: #29706 appends an upgrade-guide entry at the same spot, so one of the two will need a small rebase once the other merges.

Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Generated files are out of date

The CI build regenerated files that are not committed on this PR.

If Allow edits from maintainers is enabled, a maintainer bot will automatically
push a "Regen" commit to this branch shortly.

Otherwise, you can apply the patch manually — the missing changes are available
as the regen-patch artifact of that run:

gh pr checkout 29709 --repo apache/camel
gh run download 43200569748 --repo apache/camel -n regen-patch
git apply --index regen.patch && rm regen.patch pr-number
git commit -m "Regen" && git push

Alternatively, run ./etc/scripts/regen.sh locally and commit the resulting changes.
If the patch does not apply cleanly, update the branch from the base branch first —
the patch was generated against the merge with the base branch.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The regen commit adds @Deprecated to the generated DSL builder methods — that part is correct.

The remaining gap: getRetentionPolicy() and setRetentionPolicy() in InfluxDb2Endpoint.java (lines 141 and 145 in the current file) are still not annotated. @Deprecated on the private field does not propagate to the accessor methods — Java callers who use the endpoint API directly will not receive a compiler warning.

As noted in the existing reviews (gnodet-bot and @davsclaus), please add @Deprecated and a @deprecated Javadoc tag to both accessors:

/**
 * @deprecated Never read by the component: the InfluxDB 2.x write API has no retention policy,
 *             retention is a property of the bucket.
 */
@Deprecated
public String getRetentionPolicy() {
    return retentionPolicy;
}

/**
 * @deprecated Never read by the component: the InfluxDB 2.x write API has no retention policy,
 *             retention is a property of the bucket.
 */
@Deprecated
public void setRetentionPolicy(String retentionPolicy) {
    this.retentionPolicy = retentionPolicy;
}

Everything else in this PR — the field annotation, the constant deprecation, the upgrade guide entry, the generated catalog JSON, and the DSL builder — looks correct.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TL;DR: thanks for the regen, it is exactly what CI produced (formatter alignment plus @Deprecated on the generated influxdbRetentionpolicy() header method). Two things are left: the accessors, and a rebase.

[Important] The accessors are still not deprecated (InfluxDb2Endpoint.java:141 and :145): getRetentionPolicy() and setRetentionPolicy() have neither @Deprecated nor a @deprecated Javadoc tag, so Java callers get no compiler warning. This is the point from my earlier review and from gnodet-bot's two threads; the apiKeyHeader precedent in DoclingConfiguration (#29591) shows the shape:

/**
 * @deprecated never read: the InfluxDB 2.x write API has no retention policy, retention is a property of the bucket
 */
@Deprecated
public String getRetentionPolicy() {

(and the same for the setter).

[Important] Merge conflict in the upgrade guide: main has since appended several entries at the end of camel-4x-upgrade-guide-4_24.adoc, so the PR no longer merges cleanly. A rebase on main fixes it; keep your section after the last one. The other files merge on their own (main only corrected the adjacent operation description in #29646), but rebuilding the module after the rebase makes sure the generated files stay in sync.

[Minor] Optional, since you will touch the file anyway: replacing the description with "(deprecated) Never read: ..." while the deprecationNote also says "Never read by the component: ..." makes the generated text say it twice ("(deprecated) Never read: ... Deprecation note: Never read by the component: ..."). The docling precedent keeps the original description and puts the explanation only in the deprecationNote; the catalog already marks the option and the header as deprecated.

Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

Dhairya-007 and others added 3 commits November 12, 2026 19:09
The retentionPolicy endpoint option and the camelInfluxDB.RetentionPolicy
header are both declared and documented, and nothing reads either of them.

A retention policy is an InfluxDB 1.x concept: its write API takes one per
write, and camel-influxdb does pass it, from the same header name. The
InfluxDB 2.x API this component calls has no such parameter - retention
belongs to the bucket - so InfluxDb2Producer never asks for either value.
The header even kept the 1.x spelling, camelInfluxDB.RetentionPolicy, while
every other header of this component is CamelInfluxDB2..., which is what
gives the copy away.

Deprecate both with a note, as camel-aws2-s3vectors' unused headers and
camel-tensorflow-serving's target header are deprecated, and say in the
description that they are never read. The catalog metadata, the endpoint DSL
builder and the 4.24 upgrade guide follow.
Review follow-up on apache#29709.

@deprecated on the private field does not reach getRetentionPolicy and
setRetentionPolicy, so a caller using the endpoint API directly got no
compiler warning. Both now carry @deprecated and a @deprecated tag, as
DoclingConfiguration's apiKeyHeader does.

The descriptions go back to their original text. Replacing them with
"(deprecated) Never read: ..." while the deprecationNote also says "Never read
by the component: ..." made the generated text say it twice; the docling
precedent keeps the description and puts the explanation only in the note, and
the catalog already marks both as deprecated. Both JSON copies and the two DSL
javadoc blocks follow.
@Dhairya-007
Dhairya-007 force-pushed the fix/influxdb2-dead-retention-policy branch from f2a364b to 922728f Compare November 12, 2026 13:41
@Dhairya-007

Copy link
Copy Markdown
Contributor Author

All three points are in, thanks — and sorry it took a second pass on the accessors; the bot raised it twice and I missed that @Deprecated on a private field does not reach them.

The accessors are deprecated. getRetentionPolicy() and setRetentionPolicy() now both carry @Deprecated and a @deprecated tag, in the DoclingConfiguration#apiKeyHeader shape:

/**
 * @deprecated Never read by the component: the InfluxDB 2.x write API has no retention policy, retention is a
 *             property of the bucket
 */
@Deprecated
public String getRetentionPolicy() {

Rebased on main. The only conflict was the one you predicted, in camel-4x-upgrade-guide-4_24.adoc: main has since appended the camel-docling OCR-headers entry. Resolved by keeping that one first and my section after it. git merge-tree is clean now.

The doubled wording is gone — good catch, it read badly. Both descriptions go back to their original text ("Define the retention policy to the data created by the endpoint." and "The string that defines the retention policy to the data created by the endpoint"), so the generated text is now the docling shape: the original description plus Deprecation note: … once, rather than saying "never read" twice. Propagated to both JSON copies and the two DSL javadoc blocks — the option method and the influxdbRetentionpolicy() header method, which shared one wrapped paragraph before and now differ. The two JSON copies are byte-identical afterwards.

The @Deprecated the regen added to influxdbRetentionpolicy() is untouched.

JIRA — still no credentials for issues.apache.org here; a committer creating the ticket (or giving me the number) is all I need to retitle and amend.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Following up on my earlier review — all previous findings have been addressed.

  • ✅ getRetentionPolicy() and setRetentionPolicy() now carry @Deprecated and a @deprecated Javadoc tag — Java callers will get the compiler warning.
  • ✅ Description restored to original text; deprecation note appears once in the generated JSON (matching the tensorflow-serving precedent).
  • ✅ Rebase on main resolved cleanly.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@oscerd oscerd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean, complete deprecation.

The rationale holds: retentionPolicy and the camelInfluxDB.RetentionPolicy header are an InfluxDB 1.x idea (its write API takes a retention policy per write, and camel-influxdb does pass it), but the 2.x write API has none, retention being a property of the bucket, and InfluxDb2Producer never reads either. Deprecating both the option and the header with a note that says exactly that is right.

The regeneration is complete: deprecated: true plus the deprecationNote land in both the component META-INF JSON and the camel-catalog copy, and the endpoint DSL factory carries it too. @Deprecated is on the field, the getter, the setter and the RETENTION_POLICY constant, and the upgrade guide has an entry. The getter and setter each carry a single @deprecated javadoc block, so the generated docs stay clean.

Good.

Claude Code on behalf of oscerd

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.

5 participants