Add "create-service-key" to cloud controller, based on story #87057732 #339
Conversation
|
Hey wsxiaozhang! Thanks for submitting this pull request! All pull request authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate). When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization. |
|
We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/90769332. |
| def after_initialize | ||
| super | ||
| self.guid ||= SecureRandom.uuid | ||
| puts self.guid |
There was a problem hiding this comment.
It looks like this was left behind from debugging. Can we delete this?
|
Thanks @dsabeti for the quick comments. We will try to address them all today and resubmit. Best, max |
|
@dsabeti , thanks for the comments and we have resbumitted the PR according to your suggestions. For "orphan mitigation" and "delete keys" and "details" relevant code and test cases, we will add them in relevant stories later. |
|
Current PR depends on another PR in errors project: cloudfoundry-attic/errors#18, this will lead to the failure of Travis CI build. |
| context 'when the service instance is invalid' do | ||
| context 'because service_instance_guid is invalid' do | ||
| before do | ||
| service.save |
There was a problem hiding this comment.
I think this before block is unnecessary because we're not saving any new data to the service. We could probably delete it.
|
@dsabeti , have removed the useless lines in service_keys_controller_spec, please take a look at the latest patch |
|
We've merged in the PR for vendor/errors, so you can now update this PR to include the correct submodule reference. |
| end | ||
|
|
||
| context 'because the service instance is destroyed after controller validation and before binding save' do | ||
| let(:service_instance_guid) { 'THISISWRONG' } |
There was a problem hiding this comment.
This test is exactly the same as the test above it. Let's remove the dup. I understand that you copied this unnecessary test from ServiceBindingsController, but we're going to remove it from that test as well.
80cc598 to
27446f3
Compare
|
|
||
| many_to_one :service_instance | ||
|
|
||
| export_attributes :name, :service_instance_guid, :credentials, :syslog_drain_url |
There was a problem hiding this comment.
syslog_drain_url is not applicable to service_keys (it only matters for bindings with apps). Please remove all fields and logic regarding syslog_drain_url
| end | ||
|
|
||
| describe 'Serialization' do | ||
| it { is_expected.to export_attributes :name, :service_instance_guid, :credentials, :syslog_drain_url } |
|
Hi @xingzhou . We've made a complete pass through the code, and this should be ready to merge once you address these last few concerns. Sorry about the endless back-and-forth. Two things to note:
|
Add the rest API controller and DB model for "create service key", this patch includes the following part: 1. 1 new rest controller for "create-service-key" 2. DB migration script for adding a new table to record service keys 3. 1 new DB model for service key 4. Changes the v2 service broker client to support create service key This patch is submitted to implement story #87057732 in Service Key API More test cases will be added in other commits Signed-off-by: Tom Xing <xingzhou@cn.ibm.com>
|
@dsabeti, thanks for the review, it's the fast way for us to pick up the CC code by these back-and-forth review process, we will keep these in mind in the following development. |
Add "create-service-key" to cloud controller, based on story #87057732
Add the rest API controller and DB model for "create service key", this patch includes the following part:
This patch is submitted to implement story #87057732 in Service Key API
More test cases will be added in other commits
Signed-off-by: Tom Xing xingzhou@cn.ibm.com