Skip to content

Remove unsupported params from API calls on the UI#6686

Merged
DaanHoogland merged 1 commit intoapache:mainfrom
scclouds:remove-unecessary-params-from-apis-ui
Aug 31, 2022
Merged

Remove unsupported params from API calls on the UI#6686
DaanHoogland merged 1 commit intoapache:mainfrom
scclouds:remove-unecessary-params-from-apis-ui

Conversation

@BryanMLima
Copy link
Copy Markdown
Contributor

Description

Currently, the ACS has a lot of API call with unsupported parameters. While this does not cause any problems, as any unsupported parameters are discarded, there are a lot of messages with similar content: WARN ... Received unknown parameters for command listZones. Unknown parameters : listall. Therefore, it can be confusing for beginner operators. Moreover, if the API does not support the parameter, it should not be used regardless.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

This was tested in a local lab traversing the UI while monitoring the Management logs if there were any WARN ... Received unknown parameters for command <API>. Unknown parameters : <param>.

@acs-robot
Copy link
Copy Markdown

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@blueorangutan
Copy link
Copy Markdown

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6686 (SL-JID-2247)

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 29, 2022

Codecov Report

Merging #6686 (5d8d237) into main (9f7e0cc) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #6686      +/-   ##
============================================
- Coverage      5.87%    5.87%   -0.01%     
+ Complexity     3936     3934       -2     
============================================
  Files          2454     2454              
  Lines        242683   242683              
  Branches      37978    37978              
============================================
- Hits          14265    14261       -4     
- Misses       226839   226843       +4     
  Partials       1579     1579              
Impacted Files Coverage Δ
...apache/cloudstack/syslog/AlertsSyslogAppender.java 56.49% <0.00%> (-2.26%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm
tested in qa

Copy link
Copy Markdown

@utchoang utchoang left a comment

Choose a reason for hiding this comment

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

LGTM!

@DaanHoogland DaanHoogland merged commit c581181 into apache:main Aug 31, 2022
@yadvr yadvr added this to the 4.18.0.0 milestone Aug 31, 2022
@yadvr
Copy link
Copy Markdown
Member

yadvr commented Aug 31, 2022

cc @shwstppr @DaanHoogland could cause potential regression as some APIs may require listall passed when account is not an admin account. Has anybody verified and tested this for normal account and domain admin if/wherever applicable?

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Aug 31, 2022

cc @DaanHoogland milestone missing

neogismm pushed a commit to neogismm/cloudstack that referenced this pull request Sep 5, 2022
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.

6 participants