Add state_reason in Stack object#4823
Conversation
rashedkvm
force-pushed
the
state-reason
branch
from
1652721 to
0810cda
Compare
rashedkvm
force-pushed
the
state-reason
branch
from
0ba8c46 to
b712ee1
Compare
rashedkvm
force-pushed
the
state-reason
branch
3 times, most recently
from
ec3648c to
27197fe
Compare
sethboyles
left a comment
sethboyles
left a comment
There was a problem hiding this comment.
This LGTM, but I'd like someone involved with the RFC to take a look too.
rashedkvm
force-pushed
the
state-reason
branch
from
27197fe to
5c11479
Compare
app/messages/stack_create_message.rb
Outdated
| validates :name, presence: true, length: { maximum: 250 } | ||
| validates :description, length: { maximum: 250 } | ||
| validates :state, inclusion: { in: StackStates::VALID_STATES, message: "must be one of #{StackStates::VALID_STATES.join(', ')}" }, allow_nil: false, if: :state_requested? | ||
| validates :state_reason, length: { maximum: 1000 }, allow_nil: true |
There was a problem hiding this comment.
Can we maybe increase that to e.g. 5k ? The reson is anyway only setable by a platform admin and not a user so chances of misuse are low.
I just thought when someone wants to include a URL they can be quite lengthy sometimes, depending where they are hosted i have docu links that have around 500 chars at times. So just a bit more headroom maybe that admins dont run into the situation they have to care about link shorteners or something to their docu ? I think the VARCHAR limit of mariadb is 65k and psql has non afaik so it should be pretty safe, wdyt ?
There was a problem hiding this comment.
Yes this makes sense, I will update state_reason field length to 5000.
See RFC#0045 (Stack Management) Includes state_reason in Stack Validation Error and Warning Signed-off-by: Rashed Kamal <rashed.kamal@broadcom.com>
Signed-off-by: Rashed Kamal <rashed.kamal@broadcom.com>
Signed-off-by: Rashed Kamal <rashed.kamal@broadcom.com>
Signed-off-by: Rashed Kamal <rashed.kamal@broadcom.com>
rashedkvm
force-pushed
the
state-reason
branch
from
5c11479 to
d454ba3
Compare
Signed-off-by: Rashed Kamal <rashed.kamal@broadcom.com>
rashedkvm
force-pushed
the
state-reason
branch
from
d454ba3 to
23cb715
Compare
3 participants
Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:
A short explanation of the proposed change:
Add state_reason in Stack object
An explanation of the use cases your change solves
See RFC#0045 (Stack Management)
Includes state_reason in Stack Validation Error and Warning. See RFC#0045 (Stack Management). Includes state_reason in Stack Validation Error and Warning
Links to any other associated PRs
Enhance stack handling with state_reason field community#1420
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests