Merge pull request #338 from Temikus/fix_sql

Fixing SQL

Author: Temikus

.rubocop.yml

@@ -19,6 +19,10 @@ Style/FrozenStringLiteralComment:
 Style/HashSyntax:
   EnforcedStyle: no_mixed_keys
 
+# Do not enforce %i syntax
+Style/SymbolArray:
+  Enabled: false
+
 # HoundCI config
 
 AllCops:

CHANGELOG.md

@@ -6,17 +6,29 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 ### User-facing
 
+#### Changed
+
+- \#338 `Fog::Google::SQL` resources are now created and destroyed synchronously by default. 
+You can override it in a standard manner by passing a parameter to async method, e.g.:
+ `Fog::Google::SQL::Instance.create(true)`
+
 #### Added
 
 - \#361 `Fog::Compute::Google::Server` now recognises `network_ip` attribute to specify internal IP [mattimatti]
 
 #### Fixed
 
+- \#338 Fixed SQL Users model workflow
 - \#359 Fix whitespace escaping in XML Storage methods [temikus]
 - \#366 Fixing `Server` model to properly accept `:private_key_path` and `:public_key_path` attributes again. [temikus]
 
 ### Development changes
 
+#### Added
+
+- \#338 Major refactor of SQLv1 and SQLv2 tests + a lot of small test fixes/improvements
+   (see PR/commit messages for full set of changes) [temikus]
+
 #### Fixed
 
 - \#363 Fixed flaky Monitoring tests [temikus]

CONTRIBUTING.md

@@ -65,6 +65,13 @@ Then you can run all the live tests:
 $ rake test
 ```
 
+or just one API:
+
+```
+$ rake test:compute
+```
+(See `rake -T` for all available tasks)
+
 or just one file:
 
 ```shell
@@ -79,7 +86,8 @@ For more information on transition, read [#50](https://github.com/fog/fog-google
 
 #### Continuous integration
 
-Currently Google maintains a [Concourse CI](https://concourse-ci.org/) server, running a pipeline defined in `ci` folder. It automatically runs all integration tests against every pull-request marked with `integration` label.
+Currently Google maintains a [Concourse CI](https://concourse-ci.org/) server, running a pipeline defined in `ci` folder. 
+It automatically runs all integration tests against every pull-request marked with `integration` label.
 
 For more information on the pipeline please refer to the [ci README](https://github.com/fog/fog-google/blob/master/ci/README.md).
 

MIGRATING.md

@@ -0,0 +1,11 @@
+## fog-google 2.0 -> 3.0:
+
+### SQL:
+- `Fog::Google::SQL::Instance` resources are created synchronously by default. 
+You can override it with `Fog::Google::SQL::Instance.create(true)`
+
+- `Fog::Google::SQL::Instance.destroy` - `async:` named parameter has been replaced with `async` positional parameter.
+You should now call `Fog::Google::SQL::Instance.destroy(false)` to disable async flag, as with other Fog::Google models.
+
+- `Fog::Google::SQL::User.destroy` - `async:` named parameter has been replaced with `async` positional parameter.
+  You should now call `Fog::Google::SQL::User.destroy(false)` to disable async flag, as with other Fog::Google models.

README.md

@@ -8,6 +8,7 @@ The main maintainers for the Google sections are @icco, @Temikus and @plribeiro3
 
 **As of v1.0.0, fog-google includes google-api-client as a dependency**
 
+**See [MIGRATING.md](MIGRATING.MD) for migration between major versions**
 
 ## Storage
 
@@ -65,9 +66,22 @@ $ gem install fog-google
 
 ## Testing
 
-The tests in `tests` are deprecated. We are currently working on a migration of tests to `minitest`.
+Integration tests can be kicked off via following rake tasks.
+**Important note:** As those tests are running against real API's YOU WILL BE BILLED.
 
-For your test to be tested with real credentials, a repo maintainer may add the label `integrate` to your PR to run integration tests.
+```
+rake test               # Run all integration tests
+rake test:parallel      # Run all integration tests in parallel
+
+rake test:compute       # Run Compute API tests
+rake test:monitoring    # Run Monitoring API tests
+rake test:pubsub        # Run PubSub API tests
+rake test:sql           # Run SQL API tests
+rake test:storage       # Run Storage API tests
+```
+
+Since some resources can be expensive to test, we have a self-hosted CI server.
+Due to security considerations a repo maintainer needs to add the label `integrate` to kick off the CI.
 
 ## Setup
 

ci/pipeline.yml

@@ -1,5 +1,6 @@
 jobs:
   - name: integration-tests-compute
+    serial: true
     plan:
       - get: fog-google-src
         resource: pull-request
@@ -15,8 +16,14 @@ jobs:
           google_project: {{google_project}}
           google_json_key_data: {{google_json_key_data}}
           google_client_email: {{google_client_email}}
-
+        on_failure:
+          put: pull-request
+          params:
+            path: fog-google-src
+            status: failure
+            
   - name: integration-tests-monitoring
+    serial: true
     plan:
       - get: fog-google-src
         resource: pull-request
@@ -32,8 +39,14 @@ jobs:
           google_project: {{google_project}}
           google_json_key_data: {{google_json_key_data}}
           google_client_email: {{google_client_email}}
+        on_failure:
+          put: pull-request
+          params:
+            path: fog-google-src
+            status: failure
 
   - name: integration-tests-pubsub
+    serial: true
     plan:
       - get: fog-google-src
         resource: pull-request
@@ -49,8 +62,14 @@ jobs:
           google_project: {{google_project}}
           google_json_key_data: {{google_json_key_data}}
           google_client_email: {{google_client_email}}
+        on_failure:
+          put: pull-request
+          params:
+            path: fog-google-src
+            status: failure
 
   - name: integration-tests-sql
+    serial: true
     plan:
       - get: fog-google-src
         resource: pull-request
@@ -66,8 +85,14 @@ jobs:
           google_project: {{google_project}}
           google_json_key_data: {{google_json_key_data}}
           google_client_email: {{google_client_email}}
+        on_failure:
+          put: pull-request
+          params:
+            path: fog-google-src
+            status: failure
 
   - name: integration-tests-storage
+    serial: true
     plan:
       - get: fog-google-src
         resource: pull-request
@@ -83,6 +108,11 @@ jobs:
           google_project: {{google_project}}
           google_json_key_data: {{google_json_key_data}}
           google_client_email: {{google_client_email}}
+        on_failure:
+          put: pull-request
+          params:
+            path: fog-google-src
+            status: failure
 
   - name: github-pr-aggregator
     plan:

lib/fog/compute/google/models/ssl_certificate.rb

@@ -2,7 +2,7 @@ module Fog
   module Compute
     class Google
       ##
-      # Represents a Subnetwork resource
+      # Represents a SslCertificate resource
       #
       # @see https://cloud.google.com/compute/docs/reference/latest/sslCertificates
       class SslCertificate < Fog::Model

lib/fog/google/models/sql/instance.rb

@@ -85,14 +85,21 @@ def clone(destination_name, async: true, log_filename: nil, log_position: nil)
 
         ##
         # Creates a Cloud SQL instance
+        # @param [Boolean] async If the operation must be performed asynchronously
+        #
+        #   This is true by default since SQL instances return Google::Apis::ClientError: invalidState
+        #   whenever an instance is in a transition process (creation, deletion, etc.) which makes it
+        #   hard to operate unless one puts guard clauses on Google::Apis::ClientError everywhere.
+        #
+        #   TODO: Rethink this when API graduates out of beta. (Written as of V1beta4)
         #
         # @return [Fog::Google::SQL::Instance] Instance resource
-        def create
+        def create(async = false)
           requires :identity
 
           data = service.insert_instance(identity, attributes[:tier], attributes)
           operation = Fog::Google::SQL::Operations.new(:service => service).get(data.name)
-          operation.wait_for { !pending? }
+          operation.wait_for { ready? } unless async
           reload
         end
 
@@ -107,11 +114,19 @@ def database_flags
         ##
         # Deletes a Cloud SQL instance
         #
-        # @param [Boolean] :async If the operation must be performed asynchronously (true by default)
+        # @param [Boolean] async If the operation must be performed asynchronously (false by default)
+        #   See Fog::Google::SQL::Instance.create on details why default is set this way.
+        #
         # @return [Fog::Google::SQL::Operation] A Operation resource
-        def destroy(async: nil)
+        def destroy(async = false)
           requires :identity
 
+          # TODO(2.0): Add a deprecation warning here, depending on the decision in #27
+          # This is a compatibility fix leftover from breaking named parameter change
+          if async.is_a?(Hash)
+            async = async[:async]
+          end
+
           data = service.delete_instance(identity)
           operation = Fog::Google::SQL::Operations.new(:service => service).get(data.name)
           operation.tap { |o| o.wait_for { ready? } unless async }

lib/fog/google/models/sql/user.rb

@@ -15,9 +15,15 @@ class User < Fog::Model
         attribute :kind
         attribute :project
 
-        def destroy(async: true)
+        def destroy(async = true)
           requires :instance, :name, :host
 
+          # TODO(2.0): Add a deprecation warning here, depending on the decision in #27
+          # This is a compatibility fix leftover from breaking named parameter change
+          if async.is_a?(Hash)
+            async = async[:async]
+          end
+
           resp = service.delete_user(instance, host, name)
           operation = Fog::Google::SQL::Operations.new(:service => service).get(resp.name)
           operation.wait_for { ready? } unless async
@@ -30,9 +36,9 @@ def save(password: nil)
           data = attributes
           data[:password] = password unless password.nil?
           if etag.nil?
-            resp = service.update_user(instance, data)
-          else
             resp = service.insert_user(instance, data)
+          else
+            resp = service.update_user(instance, data)
           end
 
           operation = Fog::Google::SQL::Operations.new(:service => service).get(resp.name)

lib/fog/google/requests/sql/insert_instance.rb

@@ -7,8 +7,8 @@ class SQL
       # @see https://cloud.google.com/sql/docs/mysql/admin-api/v1beta4/instances/insert
 
       class Real
-        def insert_instance(name, tier, instance = {})
-          instance = ::Google::Apis::SqladminV1beta4::DatabaseInstance.new(instance)
+        def insert_instance(name, tier, options = {})
+          instance = ::Google::Apis::SqladminV1beta4::DatabaseInstance.new(options)
           instance.name = name
           instance.settings = ::Google::Apis::SqladminV1beta4::Settings.new(instance.settings || {})
           instance.settings.tier = tier