incubator-streampark icon indicating copy to clipboard operation
incubator-streampark copied to clipboard

Published 20 hours ago • verified publisher icon apache

[WIP][ISSUE-3007][Improve] Introduce a premise utils to reduce conditions check and exceptions processing

Open zzzk1 opened this issue 1 year ago • 6 comments

What changes were proposed in this pull request

Issue Number: close #3007

Brief change log

Add a tool to unify pre-checking and throwing exceptions

Verifying this change

This change added tests and can be verified as follows:

  • Added Test to verify the change.

Does this pull request potentially affect one of the following parts

  • Dependencies (does it add or upgrade a dependency): (no)

zzzk1 avatar Jan 26 '24 01:01 zzzk1

I really like this improvement and I'm looking forward to your next submit. 😊

wolfboys avatar Jan 26 '24 05:01 wolfboys

I will start reviewing this PR if no one reviews it

caicancai avatar Feb 05 '24 01:02 caicancai

In the current code, the backend still returns error messages to the frontend as before. The key issue is that these error messages are hard-coded and not processed through i18n. You can check out this issue. My suggestion is that after the backend detects an error, it should return a business state code, and the message info for each status code can be handled by the frontend.

wolfboys avatar Feb 05 '24 05:02 wolfboys

@RocMarshal If you have time, can you help me with the review? Thank you.

caicancai avatar Feb 19 '24 14:02 caicancai

@RocMarshal @wolfboys I will replaced the hard-coded place in the project soon

zzzk1 avatar Feb 25 '24 15:02 zzzk1

hi: There are some conflicts that need to be resolved first

wolfboys avatar Mar 09 '24 10:03 wolfboys

In the current code, the backend still returns error messages to the frontend as before. The key issue is that these error messages are hard-coded and not processed through i18n. You can check out this issue. My suggestion is that after the backend detects an error, it should return a business state code, and the message info for each status code can be handled by the frontend.

+1 for me.

Hi @zzzk1 Thank you for the pr and sorry for the late reply. thx for @wolfboys @caicancai comments.

How about

  • Only introduce a separate XXXUtils in console-service to perform a check with built-in exception binding
  • And keep common related checks in Utils.

?

I'd like to more ideas about it~

RocMarshal avatar Mar 14 '24 04:03 RocMarshal

In the current code, the backend still returns error messages to the frontend as before. The key issue is that these error messages are hard-coded and not processed through i18n. You can check out this issue. My suggestion is that after the backend detects an error, it should return a business state code, and the message info for each status code can be handled by the frontend.

+1 for me.

Hi @zzzk1 Thank you for the pr and sorry for the late reply. thx for @wolfboys @caicancai comments.

How about

  • Only introduce a separate XXXUtils in console-service to perform a check with built-in exception binding
  • And keep common related checks in Utils.

?

I'd like to more ideas about it~

  • Only introduce a separate XXXUtils in console-service to perform a check with built-in exception binding
  • And keep common related checks in Utils.

+1, This is a good idea

wolfboys avatar Mar 14 '24 05:03 wolfboys