-
Notifications
You must be signed in to change notification settings - Fork 163
970 data warhouse connection #12241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
970 data warhouse connection #12241
Conversation
app/jobs/reports/base_report.rb
Outdated
|
|
||
| def self.data_warehouse_transaction_with_timeout | ||
| original_config = ActiveRecord::Base.connection_pool.db_config.configuration_hash | ||
| ActiveRecord::Base.establish_connection(:data_warehouse) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to use a block like we do in transaction_with_timeout, ActiveRecord::Base.connected_to(shard: : data_warehouse) do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data_warehouse technically isn't a shard or replica. I was trying configure it to do some auto switching but I couldn't make it happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing usage of the :shard option for the read replica is also not technically a shard 😅
It's similar to the existing call that's in there now, but the block handles reverting to the original role/connection at the end, which I think is preferable to the more complex switching back and forth with original_config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll give it a try
e3a97e9 to
b727bd9
Compare
config/database.yml
Outdated
| sslrootcert: '/usr/local/share/aws/rds-combined-ca-bundle.pem' | ||
| migrations_paths: db/worker_jobs_migrate | ||
| <% if IdentityConfig.store.data_warehouse_enabled && IdentityConfig.store.data_warehouse_v3_enabled %> | ||
| <% redshift_config = JSON.parse(Identity::Hostdata.config_builder.secrets_client.get_secret_value(secret_id: Identity::Hostdata.config.redshift_secret_arn).secret_string) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should avoid network calls in YAML files if possible, is there a place we can put this in the config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to happen at build. We have permissions for idp to access this cross account secret. Maybe there is a way to update the configuration connection post build. Another way would be duplicating the values from the analytics account but that can cause some issues too. Do you have any suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a couple ways to access Secrets Manager via configuration in identity-hostdata and application.yml: https://gitlab.login.gov/lg/identity-devops/-/wikis/IDP-Secrets-Manager-and-Configuration. This would be the first cross-account usage, so it might require some updates for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I actually did have a solution already but I opted for this one. I'll just make a DATA_WAREHOUSE_BUILDER config.
0a226a1 to
2852770
Compare
46c8b8b to
7e0d130
Compare
828f4cd to
c3d7d69
Compare
f657a4e to
6b36de9
Compare
|
@mitchellhenke I've updated this to be more polished. |
c1ed835 to
7c355a4
Compare
7c355a4 to
b46443a
Compare
update config update connector update identity config LG-15974: Add document_type to in_person_enrollments table (#12096) Used to pass the type of ID to downstream IPP vendor Sub-task: LG-16126 [skip changelog] files Add table to record user confirmation of duplicate profiles (#12081) changelog: Upcoming Features, One Account, Add table to record user confirmation of duplicate profiles LG-15974: Add document_type to in_person_enrollments table (#12096) Used to pass the type of ID to downstream IPP vendor Sub-task: LG-16126 [skip changelog] files LG-15974: Add document_type to in_person_enrollments table (#12096) Used to pass the type of ID to downstream IPP vendor Sub-task: LG-16126 [skip changelog] config turn off config turn off config update turn off health check update config stop data-warehouse undo gem blank model turn on the WAR HOUSE gemfile lock update typo turn off warehouse turn off remove dw ref undo adapter redshift again update the WARHOUSE use store update config redshift maybe report house make instance update prefix update configuration hash ssl mode root cert, search paths rm ssl rm unused controller
b46443a to
55abe00
Compare
🎫 Ticket
Link to the relevant ticket:
https://gitlab.login.gov/lg-teams/Team-Data/data-warehouse-ag/-/issues/970
🛠 Summary of changes
This adds a data warehouse connection to redshift and one report utilizing the connection. It's feature flagged under the data_warehouse_enabled and data_warehouse_v3_enabled IdentityConfig's