You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/02/05 05:16:58 UTC
[GitHub] [flink] stevenzwu opened a new pull request #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
stevenzwu opened a new pull request #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
dawidwys commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582302171
I agree with you in principle. I am also sceptical about this change for the same reasons. My take is that `Configuration` was certainly not designed to be extensible.
I think simply using `ReadableConfig` is not a solution to @stevenzwu use case as it is used only in a couple of locations. Majority of the code still uses Configuration. Similar approach to `ReadableConfigToConfigurationAdapter` could work but this has also limitation (that's why it is internal). It is not writable/it does not give access to all the keys/map of string based properties etc.). This would be my suggested way nevertheless. I think this can achieve the same goal as making the `getRawValue` method protected. It certainly requires more code on the user side, as basically the whole `Configuration` has to be reimplemented.
To sum up, I would not include this change as it creates a contract we rather don't want to maintain while the same goal can be achieved in a different way .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582244871
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 4bde2dacca7e190e39b2ec81b4213471f32d113a (Wed Feb 05 05:19:50 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
* **This pull request references an unassigned [Jira ticket](https://issues.apache.org/jira/browse/FLINK-15907).** According to the [code contribution guide](https://flink.apache.org/contributing/contribute-code.html), tickets need to be assigned before starting with the implementation work.
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] aljoscha commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
aljoscha commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582294418
@dawidwys What do you think about this one? I'm skeptical because making the method `protected` increases the API surface that we would need to keep stable? Shouldn't "modern" stuff go against `ReadableConfig`?
Another option is to use `ReadableConfigToConfigurationAdapter` (which is `@Internal` right now) with an underlying `ReadableConfig`, which is a much simpler interface.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu closed pull request #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu closed pull request #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
dawidwys commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582335432
I will not oppose but it would be weird from my point of view. We will have a protected method that is not used from our code from outside of the class.
This will be quite a puzzle for future maintainers. Also from the user perspective it's quite weird as we are saying, sure use this method with this commit, but we do not guarantee that the next commit will not break it for you.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582249329
<!--
Meta data
Hash:4bde2dacca7e190e39b2ec81b4213471f32d113a Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/147493668 TriggerType:PUSH TriggerID:4bde2dacca7e190e39b2ec81b4213471f32d113a
Hash:4bde2dacca7e190e39b2ec81b4213471f32d113a Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4848 TriggerType:PUSH TriggerID:4bde2dacca7e190e39b2ec81b4213471f32d113a
-->
## CI report:
* 4bde2dacca7e190e39b2ec81b4213471f32d113a Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/147493668) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4848)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
I might have spoken too soon. It seems like we do use the setters sometimes. I need to investigate if we can get rid of them via other way.
{code}
setInteger(RestOptions.PORT, restPort)
setString(JobManagerOptions.ADDRESS, hostname)
{code}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
Let me clarify what we are doing. We extends from `SessionClusterEntrypoint` and `TaskManagerRunner` to add integration with our ecosystem during startup. We are passing our extension of Configuration class (backed by Archaius) to `TaskManagerRunner`. if we pass in `ReadableConfigToConfigurationAdapter`, would it work for Flink? Does any Flink internal code call any set APIs on `Configuration`.
```
public TaskManagerRunner(Configuration configuration, ResourceID resourceId)
```
We do use the set/addAll APIs in a few places. But I think those are unnecessary. So `ReadableConfigToConfigurationAdapter` should be ok for us, as long as Flink internal code is ok.
Also javadoc for `ReadableConfigToConfigurationAdapter` seems to indicate it is a bridge/stop-gap class. Do we want to make it public?
```
It is used to bridge some of the old public interfaces that work with {@link Configuration}
even though they should actually work with {@link ReadableConfig}.
```
Right now, we talked about two options.
1) expose `getRawValue` as protected so that user can extend
2) make `ReadableConfigToConfigurationAdapter` public
Any other option to allow user to extend Configuration?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
Let me clarify what we are doing. We extends from `SessionClusterEntrypoint` and `TaskManagerRunner` to add integration with our ecosystem during startup. We are passing our extension of Configuration class (backed by Archaius) to `TaskManagerRunner`.
```
public TaskManagerRunner(Configuration configuration, ResourceID resourceId)
```
We do use the set/addAll APIs in a few places. So `ReadableConfigToConfigurationAdapter` may not work for us.
Also javadoc for `ReadableConfigToConfigurationAdapter` seems to indicate it is a bridge/stop-gap class. Do we want to make it public?
```
It is used to bridge some of the old public interfaces that work with {@link Configuration}
even though they should actually work with {@link ReadableConfig}.
```
Any other option to allow user to extend Configuration?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
Let me clarify what we are doing. We extends from `SessionClusterEntrypoint` and `TaskManagerRunner` to add integration with our ecosystem during startup. We are passing our extension of Configuration class (backed by Archaius) to `TaskManagerRunner` e.g.. if we pass in `ReadableConfigToConfigurationAdapter`, would it work for Flink? Does any Flink internal code call any set APIs on `Configuration`.
```
public TaskManagerRunner(Configuration configuration, ResourceID resourceId)
```
It seems like we do use the set/addAll APIs in a few places. But I think those are unnecessary. So `ReadableConfigToConfigurationAdapter` should be ok for us, as long as Flink internal code is ok.
Also javadoc for `ReadableConfigToConfigurationAdapter` seems to indicate it is a bridge/stop-gap class. Do we want to make it public?
```
It is used to bridge some of the old public interfaces that work with {@link Configuration} even though they should actually work with {@link ReadableConfig}.
```
Right now, we talked about two options.
1) expose `getRawValue` as protected so that user can extend
2) make `ReadableConfigToConfigurationAdapter` public
Any other options to allow user to extend Configuration?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582249329
<!--
Meta data
Hash:4bde2dacca7e190e39b2ec81b4213471f32d113a Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/147493668 TriggerType:PUSH TriggerID:4bde2dacca7e190e39b2ec81b4213471f32d113a
Hash:4bde2dacca7e190e39b2ec81b4213471f32d113a Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4848 TriggerType:PUSH TriggerID:4bde2dacca7e190e39b2ec81b4213471f32d113a
-->
## CI report:
* 4bde2dacca7e190e39b2ec81b4213471f32d113a Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/147493668) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4848)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582249329
<!--
Meta data
Hash:4bde2dacca7e190e39b2ec81b4213471f32d113a Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/147493668 TriggerType:PUSH TriggerID:4bde2dacca7e190e39b2ec81b4213471f32d113a
Hash:4bde2dacca7e190e39b2ec81b4213471f32d113a Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4848 TriggerType:PUSH TriggerID:4bde2dacca7e190e39b2ec81b4213471f32d113a
-->
## CI report:
* 4bde2dacca7e190e39b2ec81b4213471f32d113a Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/147493668) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4848)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
I might have spoken too soon. It seems like we do use the set APIs occasionally. I need to investigate, but there is a good chance that they are unnecessary.
```
setInteger(RestOptions.PORT, restPort)
setString(JobManagerOptions.ADDRESS, hostname)
```
Also does Flink code internally call any set APIs from `Configuration` class?
maybe I should also be more specific on what we are doing. We are passing our extension of Configuration class to `TaskManagerRunner`. if we pass in `ReadableConfigToConfigurationAdapter`, would it work for Flink?
```
public TaskManagerRunner(Configuration configuration, ResourceID resourceId)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
Let me clarify what we are doing. We extends from `SessionClusterEntrypoint` and `TaskManagerRunner` to add integration with our ecosystem during startup. We are passing our extension of Configuration class (backed by Archaius) to `TaskManagerRunner`. if we pass in `ReadableConfigToConfigurationAdapter`, would it work for Flink? Does any Flink internal code call any set APIs on `Configuration`.
```
public TaskManagerRunner(Configuration configuration, ResourceID resourceId)
```
We do use the set/addAll APIs in a few places. But I think those are unnecessary. So `ReadableConfigToConfigurationAdapter` should be ok for us, as long as Flink internal code is ok.
Also javadoc for `ReadableConfigToConfigurationAdapter` seems to indicate it is a bridge/stop-gap class. Do we want to make it public?
```
It is used to bridge some of the old public interfaces that work with {@link Configuration} even though they should actually work with {@link ReadableConfig}.
```
Right now, we talked about two options.
1) expose `getRawValue` as protected so that user can extend
2) make `ReadableConfigToConfigurationAdapter` public
Any other option to allow user to extend Configuration?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582249329
<!--
Meta data
Hash:4bde2dacca7e190e39b2ec81b4213471f32d113a Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:4bde2dacca7e190e39b2ec81b4213471f32d113a
-->
## CI report:
* 4bde2dacca7e190e39b2ec81b4213471f32d113a UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582523390
@dawidwys @aljoscha I actually didn't know `ReadableConfigToConfigurationAdapter` before you brought it up. would you be ok to expose it by removing the `@Internal` annotation?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
I might have spoken too soon. It seems like we do use the setters sometimes. I need to investigate if we can get rid of them via other way.
```
setInteger(RestOptions.PORT, restPort)
setString(JobManagerOptions.ADDRESS, hostname)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-583705076
Actually, we may not need this. We can work around by just transferring properties into Flink `Configuration` by calling `Configuration#setString(String key, String value) `
Closing this for now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
I might have spoken too soon. It seems like we do use the setters occasionally. I need to investigate, but there is a good chance those are unnecessary.
```
setInteger(RestOptions.PORT, restPort)
setString(JobManagerOptions.ADDRESS, hostname)
```
Also does Flink code internally call any set APIs from `Configuration` class?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] aljoscha commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
aljoscha commented on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582316387
To make things easier, we could make the method protected but mark it as `@Internal`, meaning that we can still change it anytime we want.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
I might have spoken too soon. It seems like we do use the set APIs occasionally. I need to investigate, but there is a good chance that they are unnecessary.
```
setInteger(RestOptions.PORT, restPort)
setString(JobManagerOptions.ADDRESS, hostname)
```
Also does Flink code internally call any set APIs from `Configuration` class?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on issue #11017: [FLINK-15907] [runtime] [configuration] expose getRawValue as protect…
URL: https://github.com/apache/flink/pull/11017#issuecomment-582724007
Let me clarify what we are doing. We extends from `SessionClusterEntrypoint` and `TaskManagerRunner` to add integration with our ecosystem during startup. We are passing our extension of Configuration class (backed by Archaius) to `TaskManagerRunner`. if we pass in `ReadableConfigToConfigurationAdapter`, would it work for Flink? Does any Flink internal code call any set APIs on `Configuration`.
```
public TaskManagerRunner(Configuration configuration, ResourceID resourceId)
```
We do use the set/addAll APIs in a few places. But I think those are unnecessary. So `ReadableConfigToConfigurationAdapter` should be ok for us, as long as Flink internal code is ok.
Also javadoc for `ReadableConfigToConfigurationAdapter` seems to indicate it is a bridge/stop-gap class. Do we want to make it public?
```
It is used to bridge some of the old public interfaces that work with {@link Configuration}
even though they should actually work with {@link ReadableConfig}.
```
Right now, we talked about two options.
1) expose `getRawValue` as protected so that user can extend
2) make `ReadableConfigToConfigurationAdapter` public
Any other option to allow user to extend Configuration?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services