Skip to content

Conversation

bexsoft
Copy link
Collaborator

@bexsoft bexsoft commented Apr 28, 2020

What does this do?

Implements forms (UI) for settings page:

  • region
  • cache
  • compression
  • etcd
  • identity_openid
  • identity_ldap
  • policy_opa
  • logger_webhook
  • audit_webhook

Fixes a Memory leak in Configuration pages / Lambda Notification pages
Implements page for webhooks

NOTE: These forms are not connected to API yet

How to test

  • Go into configurations list, check the forms that work as expected without re-renders / hangs
  • Simulate a send configuration & check that all the information in the form is in the headers
  • Go into Lambda Notifications page & check that forms are working as expected

@bexsoft bexsoft added the WIP This PR is WIP and cannot be merged yet label Apr 28, 2020
@bexsoft bexsoft requested a review from dvaldivia April 28, 2020 22:55
@bexsoft bexsoft self-assigned this Apr 28, 2020
@bexsoft bexsoft requested review from Alevsk and cesnietor April 29, 2020 05:07
@bexsoft bexsoft removed the WIP This PR is WIP and cannot be merged yet label Apr 29, 2020
@bexsoft bexsoft changed the title [WIP] Configuration forms Configuration forms Apr 29, 2020
dvaldivia
dvaldivia previously approved these changes Apr 29, 2020
Copy link
Contributor

@Alevsk Alevsk left a comment

Choose a reason for hiding this comment

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

reviewing 👀

Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

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

It seems that it is not pointing to the right api. for instance here it should be something like .../configs/region
Screen Shot 2020-04-29 at 2 03 10 PM

I'm getting this when setting

{"code":405,"message":"method PUT is not allowed, but [GET] are"}

@cesnietor
Copy link
Collaborator

cesnietor commented Apr 29, 2020

When it failed on put, the loading remained:
Screen Shot 2020-04-29 at 2 05 45 PM

It doesn't need to fail:
To reproduce, just click on edit on region, then exit the component, the loading persists.

@cesnietor
Copy link
Collaborator

When the configuration has something the UI is not showing the info. Is that part of this PR?

./mc admin config get myminio region               
region name=us-west 

Screen Shot 2020-04-29 at 2 12 41 PM

@bexsoft
Copy link
Collaborator Author

bexsoft commented Apr 29, 2020

When the configuration has something the UI is not showing the info. Is that part of this PR?

./mc admin config get myminio region               
region name=us-west 
Screen Shot 2020-04-29 at 2 12 41 PM

No, This PR only include the forms, no connections are done yet

@bexsoft
Copy link
Collaborator Author

bexsoft commented Apr 29, 2020

Updated PR description

@cesnietor cesnietor self-requested a review April 30, 2020 00:51
Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

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

I think the payload is not being sent correctly key 0 is not valid, this is happening in all configurations
Screen Shot 2020-04-29 at 5 49 18 PM

@Alevsk Alevsk self-requested a review April 30, 2020 01:48
Alevsk
Alevsk previously approved these changes Apr 30, 2020
@bexsoft bexsoft dismissed stale reviews from Alevsk and dvaldivia via 62c267e April 30, 2020 01:55
Created Lists & forms for configurations in mcs
@bexsoft
Copy link
Collaborator Author

bexsoft commented Apr 30, 2020

Fixed issue with forms data
Screen Shot 2020-04-29 at 9 21 14 PM

@cesnietor cesnietor self-requested a review April 30, 2020 02:30
@bexsoft bexsoft merged commit 9df9309 into minio:master Apr 30, 2020
@bexsoft bexsoft deleted the configuration-forms branch April 30, 2020 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants