Implement cloudsql #274
Implement cloudsql #274
Conversation
|
|
||
| if (getReplicationCluster() != null) { | ||
| throw new GyroException( | ||
| "The 'replication-cluster' field is not supported for creating a new instance. It can only be configured in an update call."); |
There was a problem hiding this comment.
Rather than throwing the error, we should follow up with an update call to handle this, rather than making the user do gyro up again
There was a problem hiding this comment.
So that problem here is that there is a sort of circular dependency. To set this value, a replica of the primary instance (which is basically another DatabaseInstanceResource) needs to be created first which then needs to be added back to the primary instance.
I don't think an update call would solve the issue here unfortunately.
| databaseInstance.setSwitchTransactionLogsToCloudStorageEnabled(getSwitchTransactionLogsToCloudStorageEnabled()); | ||
| } | ||
|
|
||
| client.instances().insert(getProjectId(), databaseInstance); |
There was a problem hiding this comment.
are there other resources that might need this resource ?
Asking to know if a thought to a wait function was given
| return String.format( | ||
| "Acl Entry [Value: %s, Expiration Time: %s, Name: %s]", | ||
| getValue(), | ||
| getExpirationTime(), | ||
| getName()); |
There was a problem hiding this comment.
This will look funky if only the value is provided.
We should probably create this string conditionally
There was a problem hiding this comment.
Added a stringbuilder to construct the key
| * The start time for the daily backup configuration in UTC timezone in the 24 hour format - `HH:MM`. | ||
| */ | ||
| @Updatable | ||
| @Regex("[0-2][0-9]:[0-5][0-9]") |
There was a problem hiding this comment.
the hour regex will fail, 25-29 will be valid by this regex
|
|
||
| setBackupRetentionSettings(null); | ||
| if (model.getBackupRetentionSettings() != null) { | ||
| DbBackupRetentionSettings settings = new DbBackupRetentionSettings(); |
| public InsightsConfig toInsightsConfig() { | ||
| InsightsConfig config = new InsightsConfig(); | ||
|
|
||
| if (getQueryInsightsEnabled() != null) { |
There was a problem hiding this comment.
Given this field is a required value, this will never be a null conditional right ?
| } | ||
|
|
||
| /** | ||
| * The list of external networks that are allowed to connect to the instance using the IP. |
There was a problem hiding this comment.
doc entry with @subresource
| if (getSecondaryZone() != null) { | ||
| preference.setSecondaryZone(getSecondaryZone()); | ||
| } | ||
| if (getZone() != null) { | ||
| preference.setZone(getZone()); | ||
| } |
There was a problem hiding this comment.
None of them are required, that will create a blank class ?
There was a problem hiding this comment.
The api lets you set these to null and still has default values for them.
| if (getDay() != null) { | ||
| window.setDay(getDay()); | ||
| } | ||
|
|
||
| if (getHour() != null) { | ||
| window.setHour(getHour()); | ||
| } | ||
|
|
||
| if (getUpdateTrack() != null) { | ||
| window.setUpdateTrack(getUpdateTrack()); | ||
| } |
There was a problem hiding this comment.
Same, none of them are required will that create a blank class and process ?
| import gyro.core.resource.Diffable; | ||
| import gyro.google.Copyable; | ||
|
|
||
| public class DbMySqlReplicaConfiguration extends Diffable implements Copyable<MySqlReplicaConfiguration> { |
There was a problem hiding this comment.
Any required/updatable fields ?
There was a problem hiding this comment.
Doesn't seem like it from the sdk
| import gyro.core.resource.Diffable; | ||
| import gyro.google.Copyable; | ||
|
|
||
| public class DbOnPremisesConfiguration extends Diffable implements Copyable<OnPremisesConfiguration> { |
There was a problem hiding this comment.
Any required/updatable fields ?
There was a problem hiding this comment.
Doesn't seem like it from the sdk
|
|
||
| public PasswordValidationPolicy toPasswordValidationPolicy() { | ||
| PasswordValidationPolicy policy = new PasswordValidationPolicy(); | ||
| policy.setEnablePasswordPolicy(getEnablePasswordPolicy()); |
There was a problem hiding this comment.
Is the rest of the configuration still allowed if this is set to false ?
|
|
||
| public PscConfig toPscConfig() { | ||
| PscConfig config = new PscConfig(); | ||
| config.setPscEnabled(getPscEnabled()); |
There was a problem hiding this comment.
if this is set to false can you still set the projects ?
| * Only for Enterprise Plus edition instances. | ||
| */ | ||
| @Updatable | ||
| public String getFailoverDrReplicaName() { |
There was a problem hiding this comment.
This will probably be required, given it is the only settable field in this class
|
|
||
| public Settings toSettings() { | ||
| Settings settings = new Settings(); | ||
| settings.setSettingsVersion(getSettingsVersion()); |
There was a problem hiding this comment.
This will probably be a required field then.
Also given this is a db settings, re check to see if there are other fields that are required.
There was a problem hiding this comment.
The settings version is an output field so I don't think it's need the annotation.
It just needs to be provided on every update call.
The tier is basically the only thing required other than the name.
| config.setCanReschedule(getCanReschedule()); | ||
| config.setScheduleDeadlineTime(getScheduleDeadlineTime()); | ||
| config.setStartTime(getStartTime()); |
There was a problem hiding this comment.
Can all of these be set to null, given none of this is required
There was a problem hiding this comment.
The api doesn't seem to complain even if they are all set to null values.
No description provided.