[FLINK-29389][docs] Update documentation of JDBC and HBase lookup table for new caching options #20884
Conversation
…le for new caching options
There was a problem hiding this comment.
@PatrickRen Thanks for contribution, LGTM overall, left some minor comments.
| <td>optional</td> | ||
| <td>yes</td> | ||
| <td style="word-wrap: break-word;">NONE</td> | ||
| <td><p>Enum</p>Possible values: NONE, PARTIAL</td> |
There was a problem hiding this comment.
I think here we should list the three enum values, but explain only support the prior two strategies. WDYT?
There was a problem hiding this comment.
I'm afraid this could confuse users that we list an unavailable option in the doc, which increases the cost of explanation.
| <td>optional</td> | ||
| <td>yes</td> | ||
| <td style="word-wrap: break-word;">-1</td> | ||
| <td style="word-wrap: break-word;">NONE</td> | ||
| <td><p>Enum</p>Possible values: NONE, PARTIAL</td> |
| <td>The cache strategy for the lookup table. Currently supports NONE (no caching) and PARTIAL (caching entries on lookup operation in external database).</td> | ||
| </tr> | ||
| <tr> | ||
| <td><h5>lookup.partial-cache.max-rows</h5></td> |
There was a problem hiding this comment.
Refer to
, I think we should also explain the old compatible options instead of dropping it directly. Other deprecated options are similar.There was a problem hiding this comment.
I think keeping some deprecated options in the doc is a bit weird too, as we should "push" users to switch to new options asap. We keep these deprecated options in the code just for backward compatibility. Users can always refer to doc of previous versions to get the definition of those deprecated options.
There was a problem hiding this comment.
If users don't know the alternatives to old options, how do you push them to migrate to new options? Reading the code? This will be more expensive due to users can't easily get the old and new options mapping. Before we drop the old options in the code, I think we should make user can get the mapping easily, so the migration will be more smooth. Maybe refer to the https://nightlies.apache.org/flink/flink-docs-master/zh/docs/deployment/config/#deprecated-options?
There was a problem hiding this comment.
Thanks for the feedback! Of course users don't need to read the code to get the mapping. Both deprecated and new options are descriptive enough in the doc to be translated between. Anyhow I get your point to make the migration more smooth. I'll add a new section to show which deprecated option the new one is mapping to.
…le for new caching options (apache#20884)
…le for new caching options (#20884)
…le for new caching options (apache#20884)
What is the purpose of the change
This pull request updates documentation of JDBC and HBase lookup table for new caching options.
Brief change log
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (yes / no)Documentation