Add Instance Integrations models
What does this MR do and why?
In order to support integrations in Cells architecture we need to split existing integrations
into group/project integrations (existing integrations
table) and instance wide integrations. In this MR we define new instance integration models which are going to be used later on.
It additionally updates existing (currently empty) instance_integrations
db table to align it's schema with existing integrations
table. We previously created this table trimming any unnecessary columns, but we need them in the end in order to provide a smoother transition. Fewer changes required.
Mentions #474809 (closed)
Migration output
Up
main: == [advisory_lock_connection] object_id: 126140, pg_backend_pid: 82711
main: == 20241011103239 UpdateInstanceIntegrationTable: migrating ===================
main: -- add_column(:instance_integrations, :project_id, :bigint, {:null=>true})
main: -> 0.0042s
main: -- add_column(:instance_integrations, :group_id, :bigint, {:null=>true})
main: -> 0.0008s
main: -- add_column(:instance_integrations, :inherit_from_id, :bigint, {:null=>true})
main: -> 0.0008s
main: -- add_column(:instance_integrations, :instance, :boolean, {:default=>true})
main: -> 0.0046s
main: -- rename_column(:instance_integrations, :type, :type_new)
main: -> 0.0154s
main: -- transaction_open?(nil)
main: -> 0.0000s
main: -- transaction_open?(nil)
main: -> 0.0000s
main: -- execute("ALTER TABLE instance_integrations\nADD CONSTRAINT project_id_null_constraint\nCHECK ( project_id IS NULL )\nNOT VALID;\n")
main: -> 0.0045s
main: -- execute("SET statement_timeout TO 0")
main: -> 0.0004s
main: -- execute("ALTER TABLE instance_integrations VALIDATE CONSTRAINT project_id_null_constraint;")
main: -> 0.0007s
main: -- execute("RESET statement_timeout")
main: -> 0.0003s
main: -- transaction_open?(nil)
main: -> 0.0000s
main: -- transaction_open?(nil)
main: -> 0.0000s
main: -- execute("ALTER TABLE instance_integrations\nADD CONSTRAINT group_id_null_constraint\nCHECK ( group_id IS NULL )\nNOT VALID;\n")
main: -> 0.0007s
main: -- execute("ALTER TABLE instance_integrations VALIDATE CONSTRAINT group_id_null_constraint;")
main: -> 0.0014s
main: -- transaction_open?(nil)
main: -> 0.0000s
main: -- transaction_open?(nil)
main: -> 0.0000s
main: -- execute("ALTER TABLE instance_integrations\nADD CONSTRAINT inherit_from_id_null_constraint\nCHECK ( inherit_from_id IS NULL )\nNOT VALID;\n")
main: -> 0.0014s
main: -- execute("ALTER TABLE instance_integrations VALIDATE CONSTRAINT inherit_from_id_null_constraint;")
main: -> 0.1876s
main: -- transaction_open?(nil)
main: -> 0.0000s
main: -- transaction_open?(nil)
main: -> 0.0000s
main: -- execute("ALTER TABLE instance_integrations\nADD CONSTRAINT instance_is_true_constraint\nCHECK ( instance = TRUE )\nNOT VALID;\n")
main: -> 0.0015s
main: -- execute("ALTER TABLE instance_integrations VALIDATE CONSTRAINT instance_is_true_constraint;")
main: -> 0.0013s
main: == 20241011103239 UpdateInstanceIntegrationTable: migrated (0.2865s) ==========
main: == [advisory_lock_connection] object_id: 126140, pg_backend_pid: 82711
ci: == [advisory_lock_connection] object_id: 134180, pg_backend_pid: 83133
ci: == 20241011103239 UpdateInstanceIntegrationTable: migrating ===================
ci: -- add_column(:instance_integrations, :project_id, :bigint, {:null=>true})
ci: -> 0.0021s
ci: -- add_column(:instance_integrations, :group_id, :bigint, {:null=>true})
ci: -> 0.0052s
ci: -- add_column(:instance_integrations, :inherit_from_id, :bigint, {:null=>true})
ci: -> 0.0015s
ci: -- add_column(:instance_integrations, :instance, :boolean, {:default=>true})
ci: -> 0.0056s
ci: -- rename_column(:instance_integrations, :type, :type_new)
ci: -> 0.0162s
ci: -- transaction_open?(nil)
ci: -> 0.0000s
ci: -- transaction_open?(nil)
ci: -> 0.0000s
ci: -- execute("ALTER TABLE instance_integrations\nADD CONSTRAINT project_id_null_constraint\nCHECK ( project_id IS NULL )\nNOT VALID;\n")
ci: -> 0.0013s
ci: -- execute("SET statement_timeout TO 0")
ci: -> 0.0004s
ci: -- execute("ALTER TABLE instance_integrations VALIDATE CONSTRAINT project_id_null_constraint;")
ci: -> 0.0011s
ci: -- execute("RESET statement_timeout")
ci: -> 0.0004s
ci: -- transaction_open?(nil)
ci: -> 0.0000s
ci: -- transaction_open?(nil)
ci: -> 0.0000s
ci: -- execute("ALTER TABLE instance_integrations\nADD CONSTRAINT group_id_null_constraint\nCHECK ( group_id IS NULL )\nNOT VALID;\n")
ci: -> 0.0004s
ci: -- execute("ALTER TABLE instance_integrations VALIDATE CONSTRAINT group_id_null_constraint;")
ci: -> 0.0008s
ci: -- transaction_open?(nil)
ci: -> 0.0000s
ci: -- transaction_open?(nil)
ci: -> 0.0000s
ci: -- execute("ALTER TABLE instance_integrations\nADD CONSTRAINT inherit_from_id_null_constraint\nCHECK ( inherit_from_id IS NULL )\nNOT VALID;\n")
ci: -> 0.0025s
ci: -- execute("ALTER TABLE instance_integrations VALIDATE CONSTRAINT inherit_from_id_null_constraint;")
ci: -> 0.0006s
ci: -- transaction_open?(nil)
ci: -> 0.0000s
ci: -- transaction_open?(nil)
ci: -> 0.0000s
ci: -- execute("ALTER TABLE instance_integrations\nADD CONSTRAINT instance_is_true_constraint\nCHECK ( instance = TRUE )\nNOT VALID;\n")
ci: -> 0.0008s
ci: -- execute("ALTER TABLE instance_integrations VALIDATE CONSTRAINT instance_is_true_constraint;")
ci: -> 0.0007s
ci: == 20241011103239 UpdateInstanceIntegrationTable: migrated (0.1041s) ==========
ci: == [advisory_lock_connection] object_id: 134180, pg_backend_pid: 83133
Down
main: == [advisory_lock_connection] object_id: 126200, pg_backend_pid: 81813
main: == 20241011103239 UpdateInstanceIntegrationTable: reverting ===================
main: -- remove_column(:instance_integrations, :project_id, {:if_exists=>true})
main: -> 0.0217s
main: -- remove_column(:instance_integrations, :group_id, {:if_exists=>true})
main: -> 0.0035s
main: -- remove_column(:instance_integrations, :inherit_from_id, {:if_exists=>true})
main: -> 0.0038s
main: -- remove_column(:instance_integrations, :instance, {:if_exists=>true})
main: -> 0.0056s
main: -- rename_column(:instance_integrations, :type_new, :type)
main: -> 0.0033s
main: == 20241011103239 UpdateInstanceIntegrationTable: reverted (0.0478s) ==========
main: == [advisory_lock_connection] object_id: 126200, pg_backend_pid: 81813
ci: == [advisory_lock_connection] object_id: 126140, pg_backend_pid: 82231
ci: == 20241011103239 UpdateInstanceIntegrationTable: reverting ===================
ci: -- remove_column(:instance_integrations, :project_id, {:if_exists=>true})
ci: -> 0.0295s
ci: -- remove_column(:instance_integrations, :group_id, {:if_exists=>true})
ci: -> 0.0030s
ci: -- remove_column(:instance_integrations, :inherit_from_id, {:if_exists=>true})
ci: -> 0.0046s
ci: -- remove_column(:instance_integrations, :instance, {:if_exists=>true})
ci: -> 0.0035s
ci: -- rename_column(:instance_integrations, :type_new, :type)
ci: -> 0.0028s
ci: == 20241011103239 UpdateInstanceIntegrationTable: reverted (0.0593s) ==========
ci: == [advisory_lock_connection] object_id: 126140, pg_backend_pid: 82231
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
Merge request reports
Activity
changed milestone to %17.5
assigned to @georgekoltsov
mentioned in issue #474809 (closed)
added pipelinetier-1 label
mentioned in issue Taucher2003/reporting#750 (closed)
- A deleted user
added backend database databasereview pending featureaddition typefeature labels
2 Warnings This merge request is quite big (942 lines changed), please consider splitting it into multiple merge requests. featureaddition and featureenhancement merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the Technical Writer counterpart.
For more information, see:
- The Handbook page on merge request types.
- The definition of done documentation.
1 Message This merge request adds or changes files that require a review from the Database team. This merge request requires a database review. To make sure these changes are reviewed, take the following steps:
- Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
- Prepare your MR for database review according to the docs.
- Assign and mention the database reviewer suggested by Reviewer Roulette.
The following files require a review from the Database team:
db/migrate/20241011103239_update_instance_integration_table.rb
db/schema_migrations/20241011103239
db/docs/instance_integrations.yml
db/structure.sql
Reviewer roulette
Category Reviewer Maintainer backend @fdegier
(UTC+2, 1 hour ahead of author)
@sashi_kumar
(UTC+0, 1 hour behind author)
database @igor.drozdov
(UTC+2, 1 hour ahead of author)
@bhrai
(UTC+2, 1 hour ahead of author)
Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
Danger- A deleted user
added Data WarehouseImpact Check label
1 # frozen_string_literal: true 2 3 class UpdateInstanceIntegrationTable < Gitlab::Database::Migration[2.2] 4 milestone '17.5' 5 6 disable_ddl_transaction! 7 8 def up 9 add_column :instance_integrations, :project_id, :bigint, null: true 10 add_column :instance_integrations, :group_id, :bigint, null: true 11 add_column :instance_integrations, :inherit_from_id, :bigint, null: true 12 add_column :instance_integrations, :instance, :boolean, default: true 13 14 rename_column :instance_integrations, :type, :type_new @.luke may I ask you for an early look of this MR? I followed your suggestions. Updated instance integrations table to have the same schema as intergrations but have put db constraints on foreign key columns to always be null. Will polish it up on monday but wonder what you think so far. Thanks
@georgekoltsov It looks good! It looks like there is something not working out of the box with modules that are
prepended
, like theEnableSslVerification
which isprepended
into some integrations including Jenkins.When
super
is called here it's an empty array in the instance integrations model, whereas in the regular models it's an array ofIntegrations::Field
instances, and it causes this problem:Integrations::Instance::Jenkins.new.fields # TypeError: no implicit conversion from nil to integer # from /Users/luke/Sites/gitlab-development-kit/gitlab/app/models/concerns/integrations/enable_ssl_verification.rb:29:in `delete_at'
It seems like modules that are
included
are working fine.service_hook
is defined in the Jenkins integration throughHasWebHook
:Integrations::Instance::Jenkins.new.service_hook # => nil
But besides that, the models looked good and I didn't bump into any other problems. I can do things like:
integration = Integrations::Instance::Slack.new integration.save # => true integration.active = true integration.valid? # => false integration.errors.full_messages # => ["Webhook can't be blank", "Webhook must be a valid URL"] integration.webhook = 'https://example.com' integration.save # => true Integrations::Instance::Slack.count # => 1 Integrations::Instance::Jira.count # => 0
Edited by Luke DuncalfeThanks @.luke I managed to get it to work after a bit of struggle by doing the following
# app/models/integrations/instance_integration.rb def self.fields superclass.fields end
Without this change, the instance model's
.fields
method returns an empty array (as you highlighted) even though these fields are defined in the superclass, but callingsuper
on existing fields method doesn't fetch the defined fields from the super class, it still returns an empty array from app/models/integration.rb:201. So I updated the instance model module to fetch fields from superclass instead. Seems to work, but let me know if something's not right.I am going to add some tests in a meantime.
@.luke should be good for another look in case you spot any issues
@georgekoltsov Thanks, I couldn't spot any extra issues. What you mention in #499482 (comment 2161838709) opens a door potentially to a lot of headaches of some form or another. At least some of which I think are problems no matter which approach we take.
@bmarjanovic Could you please give this MR its backend maintainer review, and let us know your thoughts on the refactor
Great work @georgekoltsov
We're having some
pipeline, and I left one core comment before continuing, let me know what do you think
requested review from @.luke
removed review request for @.luke
note
Adding specs revealed another issue with another integration - Jira. Running the spec fails with
ActiveRecord::InvalidForeignKey: PG::ForeignKeyViolation: ERROR: insert or update on table "jira_tracker_data" violates foreign key constraint "fk_c98abcd54c" DETAIL: Key (integration_id)=(114) is not present in table "integrations".
I will probably leave this for a follow up MR.
Same for
Redmine
1) Integrations::Instance::Redmine successfully persists the record Failure/Error: connection.public_send(...) ActiveRecord::InvalidForeignKey: PG::ForeignKeyViolation: ERROR: insert or update on table "issue_tracker_data" violates foreign key constraint "fk_33921c0ee1" DETAIL: Key (integration_id)=(170) is not present in table "integrations".
Same for
Zentao
2) Integrations::Instance::Zentao behaves like instance integration successfully persists the record Failure/Error: connection.public_send(...) ActiveRecord::InvalidForeignKey: PG::ForeignKeyViolation: ERROR: insert or update on table "zentao_tracker_data" violates foreign key constraint "fk_rails_84efda7be0" DETAIL: Key (integration_id)=(199) is not present in table "integrations".
Follow up #499482 (closed)
Database migrations (on the main database)
Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).
Migration Type Total runtime Result DB size change 20241011103239 - UpdateInstanceIntegrationTable Regular 10.6 s +0.00 B Runtime Histogram for all migrations
Query Runtime Count 0 seconds - 0.01 seconds 0 0.01 seconds - 0.1 seconds 1 0.1 seconds - 1 second 16 1 second - 5 seconds 0 5 seconds - 15 seconds 0 15 seconds - 5 minutes 0 5 minutes + 0 Migration: 20241011103239 - UpdateInstanceIntegrationTable
- Type: Regular
- Duration: 10.6 s
- Database size change: +0.00 B
Calls Total Time Max Time Mean Time Rows Query 1 10.1 ms 10.1 ms 10.1 ms 0 ALTER TABLE "instance_integrations" ADD "instance" boolean DEFAULT TRUE
1 9.7 ms 9.7 ms 9.7 ms 0 ALTER TABLE "instance_integrations" ADD "project_id" bigint
1 4.9 ms 4.9 ms 4.9 ms 0 ALTER TABLE instance_integrations ADD CONSTRAINT project_id_null_constraint CHECK ( project_id IS NULL ) NOT VALID
1 2.4 ms 2.4 ms 2.4 ms 0 ALTER TABLE instance_integrations ADD CONSTRAINT inherit_from_id_null_constraint CHECK ( inherit_from_id IS NULL ) NOT VALID
1 1.7 ms 1.7 ms 1.7 ms 0 ALTER TABLE instance_integrations ADD CONSTRAINT instance_is_true_constraint CHECK ( instance = TRUE ) NOT VALID
1 1.5 ms 1.5 ms 1.5 ms 0 ALTER TABLE instance_integrations VALIDATE CONSTRAINT inherit_from_id_null_constraint
1 1.2 ms 1.2 ms 1.2 ms 0 ALTER TABLE "instance_integrations" ADD "inherit_from_id" bigint
1 1.2 ms 1.2 ms 1.2 ms 0 ALTER TABLE instance_integrations VALIDATE CONSTRAINT instance_is_true_constraint
1 1.1 ms 1.1 ms 1.1 ms 0 ALTER TABLE instance_integrations VALIDATE CONSTRAINT group_id_null_constraint
1 0.6 ms 0.6 ms 0.6 ms 0 ALTER TABLE instance_integrations ADD CONSTRAINT group_id_null_constraint CHECK ( group_id IS NULL ) NOT VALID
1 0.5 ms 0.5 ms 0.5 ms 0 ALTER TABLE "instance_integrations" ADD "group_id" bigint
1 0.5 ms 0.5 ms 0.5 ms 0 ALTER TABLE "instance_integrations" RENAME COLUMN "type" TO "type_new"
1 0.4 ms 0.4 ms 0.4 ms 0 ALTER TABLE instance_integrations VALIDATE CONSTRAINT project_id_null_constraint
1 0.1 ms 0.1 ms 0.1 ms 1 SELECT "feature_gates"."key", "feature_gates"."value" FROM "feature_gates" WHERE "feature_gates"."feature_key" = $1
2 0.0 ms 0.0 ms 0.0 ms 2 SELECT pg_backend_pid()
1 0.0 ms 0.0 ms 0.0 ms 1 SELECT $1::regtype::oid
Histogram for UpdateInstanceIntegrationTable
Query Runtime Count 0 seconds - 0.01 seconds 0 0.01 seconds - 0.1 seconds 1 0.1 seconds - 1 second 16 1 second - 5 seconds 0 5 seconds - 15 seconds 0 15 seconds - 5 minutes 0 5 minutes + 0 Other information
No other migrations pending on GitLab.com
Clone details
Clone ID Clone Created At Clone Data Timestamp Expected Removal Time database-testing-3794432-15701349-main
2024-10-15T17:53:37Z 2024-10-15T12:47:19Z 2024-10-16 06:04:03 +0000 database-testing-3794432-15701349-ci
2024-10-15T17:53:37Z 2024-10-15T16:44:58Z 2024-10-16 06:04:03 +0000 Database migrations (on the ci database)
Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).
Migration Type Total runtime Result DB size change 20241011103239 - UpdateInstanceIntegrationTable Regular 12.3 s +0.00 B Runtime Histogram for all migrations
Query Runtime Count 0 seconds - 0.01 seconds 0 0.01 seconds - 0.1 seconds 2 0.1 seconds - 1 second 14 1 second - 5 seconds 0 5 seconds - 15 seconds 0 15 seconds - 5 minutes 0 5 minutes + 0 Migration: 20241011103239 - UpdateInstanceIntegrationTable
- Type: Regular
- Duration: 12.3 s
- Database size change: +0.00 B
Calls Total Time Max Time Mean Time Rows Query 1 3.5 ms 3.5 ms 3.5 ms 0 ALTER TABLE "instance_integrations" ADD "project_id" bigint
1 2.7 ms 2.7 ms 2.7 ms 0 ALTER TABLE "instance_integrations" ADD "instance" boolean DEFAULT TRUE
1 0.6 ms 0.6 ms 0.6 ms 0 ALTER TABLE instance_integrations ADD CONSTRAINT project_id_null_constraint CHECK ( project_id IS NULL ) NOT VALID
1 0.5 ms 0.5 ms 0.5 ms 0 ALTER TABLE instance_integrations ADD CONSTRAINT group_id_null_constraint CHECK ( group_id IS NULL ) NOT VALID
1 0.5 ms 0.5 ms 0.5 ms 0 ALTER TABLE instance_integrations ADD CONSTRAINT instance_is_true_constraint CHECK ( instance = TRUE ) NOT VALID
1 0.5 ms 0.5 ms 0.5 ms 0 ALTER TABLE instance_integrations VALIDATE CONSTRAINT project_id_null_constraint
1 0.5 ms 0.5 ms 0.5 ms 0 ALTER TABLE instance_integrations ADD CONSTRAINT inherit_from_id_null_constraint CHECK ( inherit_from_id IS NULL ) NOT VALID
1 0.4 ms 0.4 ms 0.4 ms 0 ALTER TABLE instance_integrations VALIDATE CONSTRAINT group_id_null_constraint
1 0.4 ms 0.4 ms 0.4 ms 0 ALTER TABLE "instance_integrations" ADD "group_id" bigint
1 0.4 ms 0.4 ms 0.4 ms 0 ALTER TABLE instance_integrations VALIDATE CONSTRAINT instance_is_true_constraint
1 0.4 ms 0.4 ms 0.4 ms 0 ALTER TABLE "instance_integrations" ADD "inherit_from_id" bigint
1 0.4 ms 0.4 ms 0.4 ms 0 ALTER TABLE "instance_integrations" RENAME COLUMN "type" TO "type_new"
1 0.3 ms 0.3 ms 0.3 ms 0 ALTER TABLE instance_integrations VALIDATE CONSTRAINT inherit_from_id_null_constraint
2 0.0 ms 0.0 ms 0.0 ms 2 SELECT pg_backend_pid()
1 0.0 ms 0.0 ms 0.0 ms 1 SELECT $1::regtype::oid
Histogram for UpdateInstanceIntegrationTable
Query Runtime Count 0 seconds - 0.01 seconds 0 0.01 seconds - 0.1 seconds 2 0.1 seconds - 1 second 14 1 second - 5 seconds 0 5 seconds - 15 seconds 0 15 seconds - 5 minutes 0 5 minutes + 0 Other information
No other migrations pending on GitLab.com
Clone details
Clone ID Clone Created At Clone Data Timestamp Expected Removal Time database-testing-3794432-15701349-main
2024-10-15T17:53:37Z 2024-10-15T12:47:19Z 2024-10-16 06:04:03 +0000 database-testing-3794432-15701349-ci
2024-10-15T17:53:37Z 2024-10-15T16:44:58Z 2024-10-16 06:04:03 +0000
Brought to you by gitlab-org/database-team/gitlab-com-database-testing. Epic
Edited by ****added database-testing-automation label
requested review from @.luke
@igor.drozdov may I ask you to do database review of this one?
@georgekoltsov thanks, database LGTM
requested review from @igor.drozdov
changed milestone to %17.6
added missed:17.5 label
mentioned in issue #499482 (closed)
added pipeline:mr-approved label
added pipelinetier-2 label and removed pipelinetier-1 label
Before you set this MR to auto-merge
This merge request will progress on pipeline tiers until it reaches the last tier: pipelinetier-3. We will trigger a new pipeline for each transition to a higher tier.
Before you set this MR to auto-merge, please check the following:
- You are the last maintainer of this merge request
- The latest pipeline for this merge request is pipelinetier-3 (You can find which tier it is in the pipeline name)
- This pipeline is recent enough (created in the last 8 hours)
If all the criteria above apply, please set auto-merge for this merge request.
See pipeline tiers and merging a merge request for more details.
removed review request for @.luke
requested review from @bmarjanovic
Generated bygitlab_quality-test_tooling
.
Slow tests detected in this merge request. These slow tests might be related to this merge request's changes.Click to expand
Job File Name Duration Expected duration #8106543841 spec/lib/gitlab/database/decomposition/migrate_spec.rb#L103
Gitlab::Database::Decomposition::Migrate#process! when the checks pass copies main database to ci database 28.76 s < 27.12 s #8106544075 spec/features/admin/integrations/instance_integrations_spec.rb#L12
Instance integrations behaves like integration settings form displays all the integrations 114.74 s < 50.13 s #8106544024 spec/features/admin/integrations/instance_integrations_spec.rb#L12
Instance integrations behaves like integration settings form displays all the integrations 121.91 s < 50.13 s #8106544135 spec/features/admin/integrations/instance_integrations_spec.rb#L12
Instance integrations behaves like integration settings form displays all the integrations 132.82 s < 50.13 s - A deleted user
added rspec:slow test detected label
E2E Test Result Summary
allure-report-publisher
generated test report!e2e-test-on-gdk:
test report for 66b8ff81expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Plan | 76 | 0 | 0 | 0 | 76 | ✅ | | Govern | 71 | 0 | 2 | 0 | 73 | ✅ | | Create | 128 | 0 | 18 | 0 | 146 | ✅ | | Verify | 45 | 0 | 2 | 0 | 47 | ✅ | | Monitor | 8 | 0 | 0 | 0 | 8 | ✅ | | Release | 5 | 0 | 0 | 0 | 5 | ✅ | | Analytics | 2 | 0 | 0 | 0 | 2 | ✅ | | Package | 17 | 0 | 18 | 0 | 35 | ✅ | | Data Stores | 33 | 0 | 1 | 0 | 34 | ✅ | | Fulfillment | 2 | 0 | 0 | 0 | 2 | ✅ | | Secure | 4 | 0 | 0 | 0 | 4 | ✅ | | Manage | 1 | 0 | 1 | 0 | 2 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 392 | 0 | 42 | 0 | 434 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
added Data WarehouseNot Impacted label and removed Data WarehouseImpact Check label
1 1 # frozen_string_literal: true suggestion: instead of creating 50+ new identical classes, could we do something like the following instead?
module Integrations module Instance INTEGRATIONS = %w[AppleAppStore Asana Bamboo ...].freeze INTEGRATIONS.each do |integration_name| const_set(integration_name, Class.new(Object.const_get("::Integrations::#{integration_name}")) do include InstanceIntegration end) end end end
I think we shouldn't do metaprogramming here
. Where we've done metaprogramming to create classes, like in our GraphQL Global ID types I've found over time it's led to problems that would be easy to solve if they existed as normal files, allowing us to treat each one individually if we wanted to in future.@.luke I thought it would be easier to have overall control here, rather than creating 50+ integrations.
If we have a need to introduce some changes to a single class, we can always create a new one, and remove it from the
Integrations::Instance::INTEGRATIONS
constant.
added databasereviewed label and removed databasereview pending label
removed review request for @igor.drozdov
mentioned in merge request !169768 (closed)
Due to the issue discovered in #499482 (closed) we no longer are going to follow this approach. Closing this MR
Hello @georgekoltsov
The database team is looking for ways to improve the database review process and we would love your help!
If you'd be open to someone on the database team reaching out to you for a chat, or if you'd like to leave some feedback asynchronously, just post a reply to this comment mentioning:
@gitlab-org/database-team
And someone will be by shortly!
Thanks for your help!
This message was generated automatically. You're welcome to improve it.