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

[FLINK-13799][web]: fix error message web submit is disables in the c… #9504

Closed
wants to merge 1 commit into from

Conversation

vthinkxie
Copy link
Contributor

@vthinkxie vthinkxie commented Aug 22, 2019

…onfig

What is the purpose of the change

fix https://issues.apache.org/jira/browse/FLINK-13799

Brief change log

there is no method to get if web.submit.enable is false in rest API

I hide the jar error message from rest by modifying the frontend interceptor

Verifying this change

If you put web.submit.enable: false into the configuration, no errors will continuously pop up.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (no)

@flinkbot
Copy link
Collaborator

flinkbot commented Aug 22, 2019

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 291ac11 (Wed Oct 16 08:36:12 UTC 2019)

Warnings:

  • No documentation files were touched! Remember to keep the Flink docs up to date!

Mention the bot in a comment to re-run the automated checks.

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 for a full explanation of the review process.


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 Bot commands
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

@sorahn
Copy link

sorahn commented Aug 22, 2019

LGTM

@flinkbot
Copy link
Collaborator

flinkbot commented Aug 22, 2019

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run travis re-run the last Travis build

@@ -32,7 +32,7 @@ export class AppInterceptor implements HttpInterceptor {
* Error response from below url should be ignored
*/
const ignoreErrorUrlEndsList = ['checkpoints/config', 'checkpoints'];
const ignoreErrorMessage = ['File not found.'];
const ignoreErrorMessage = ['File not found.', 'Unable to load requested file /jars.'];
Copy link
Contributor

Choose a reason for hiding this comment

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

While this does cover the case of multiple error messages when accessing the page, the user still gets odd error messages if he tries to use the submit page in any way.

If we want to backport the proper solution (See subtasks of FLINK-13799) to 1.9, then I would skip this PR.
If we don't want to backport it, then we need a more extensive fix imo.

Copy link
Contributor Author

@vthinkxie vthinkxie Aug 23, 2019

Choose a reason for hiding this comment

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

the upload button in the submit page is already hidden when getting the error message, so the user could not use the submit page in the case.
I think we should wait for the proper solution if not hurry.

@zentol zentol self-assigned this Aug 22, 2019
@vthinkxie
Copy link
Contributor Author

Hi @zentol
I found that the web can get web.submit.enable from the jobmanager configuration, is that correct to judge if the submit is enabled from the key web.submit.enable?
屏幕快照 2019-09-19 下午4 42 13

@zentol
Copy link
Contributor

zentol commented Oct 2, 2019

@vthinkxie FLINK-13817 exposes whether submissions are enabled in a more reliable way. The configuration is is unreliable since it only lists explicitly configured keys; if the key is not explicitly configured you'd have to make assumptions on the default.

@zentol
Copy link
Contributor

zentol commented Oct 7, 2019

I will close this PR for the time being. If we do not manage to implement the approach outlined in FLINK-13818 in time for a release we can fallback on this proposal.

@zentol zentol closed this Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants