feat: Delete data dump from S3 after data load has been successful #600

antroy-madetech · 2024-10-17T10:32:30Z

Addresses DBTP-1109

Delete data dump from S3 after data load has been successful
Delete the old database-copy Docker files
Move the new Docker files from database-copy2 into database-copy
Added a database copy command that rolls the dump and load commands together

Checklist:

Title:

Scope included as per conventional commits
Ticket reference included (unless it's a quick out of ticket thing)

Description:

Link to ticket included (unless it's a quick out of ticket thing)
Includes tests (or an explanation for why it doesn't)
If the work includes user interface changes, before and after screenshots included in description
Includes any applicable changes to the documentation in this code base
Includes link(s) to any applicable changes to the documentation in the DBT Platform Documentation (can be to a pull request)

Tasks:

Trigger the pull request regression tests for this branch and confirm that they are passing

WillGibson · 2024-10-29T11:41:45Z

dbt_platform_helper/commands/database_helpers.py

+class MaintenancePageProvider:
+    def __init__(self):
+        self.offline = offline_command
+        self.online = online_command


 class DatabaseCopy:


How do you feel about moving the classes etc. into a new src or similar directory and starting to tidily namespace it all?

We've had similar thoughts. "commands" module will be where the thin click wrappers will live. "domain" module for the business logic and "providers" module for the common code that connects with things outside the domain logic (most of the "utils" module probably belongs in here)

(I'll write this up in the confluence doc for people to comment on)

WillGibson · 2024-10-29T11:46:04Z

dbt_platform_helper/commands/database_helpers.py

+        ] = MaintenancePageProvider(),
+        input_fn: Callable[[str], str] = click.prompt,
+        echo_fn: Callable[[str], str] = click.secho,
+        abort_fn: Callable[[str], None] = abort_with_error,


I haven't really looked at this thoroughly, but this feels like a lot of things going into the constructor.

It makes me wonder whether the DatabaseCopy class is doing a bit much. Maybe we can separate out the copying work from the input/output/side effects?

I think some of this stuff can be tidied up further. For example the input/echo/abort are all UI elements that can probably be moved into a single UserInterfaceProvider or something. Perhaps in a tidy up step after we get this command out for FFT to use.

…ocess

…tional vpc parameters

…ific log to be displayed

ksugden · 2024-10-30T10:12:43Z

dbt_platform_helper/COMMANDS.md

-
+  - The application name. Required unless you are running the command from your deploy repo
+- `--from <text>`
+  - This is required unless you are running the command from your deploy repo


Is this right? Looks from line 994 that --from is mandatory? Description should probably be:
The environment you are dumping data from

ksugden · 2024-10-30T10:21:48Z

dbt_platform_helper/commands/environment.py

-        )
-        raise click.Abort
+    maintenance_page = MaintenancePageProvider()
+    maintenance_page.offline(app, env, svc, template, vpc)


Reading this code sounds a bit like the maintenance page is what is going offline. It's probably going to confuse people. Maybe we could improve naming:

maintenance_page_provider.set_application_offline(app, env, svc, template, vpc)

or maybe:

maintenance_page.activate(app, env, svc, template, vpc)

ksugden · 2024-10-30T10:40:04Z

dbt_platform_helper/domain/database_copy.py

+            vpc_name = config.get("environments", {}).get(env, {}).get("vpc")
+        return vpc_name
+
+    def run_database_copy_task(


thoughts for later - we should have an EcsTaskService for doing this sort of thing

ksugden · 2024-10-30T10:46:20Z

images/tools/database-copy/entrypoint.sh

+  clean_up
+
+  echo "Stopping data load"
+fi


This bash script is quite unwieldy. Could we separate it into scripts for the different 'steps'? Maybe should consider something like https://aws.amazon.com/step-functions/features/?pg=ln&sec=hs in the future.

We'll look at this in the tidy up step.

ksugden · 2024-10-30T10:47:32Z

images/tools/database-copy2/Dockerfile

@@ -1,11 +0,0 @@
-FROM postgres:16


Are we able to clean up this directory?

antroy-madetech force-pushed the DBTP-1109-DataCopy_2 branch from 0b99c6e to 50287a5 Compare October 22, 2024 13:56

WillGibson reviewed Oct 29, 2024

View reviewed changes

antroy-madetech added 28 commits October 29, 2024 16:47

Removed database-copy image

f10ce2a

Moved new database copy docker image into the database-copy dir

e1a3ed6

feat: Added dump file deletion from S3 at the end of the data load pr…

b2e3576

…ocess

Added a copy command to chain the dump and load commands

5f37279

Refactoring of helper methods

7462c7f

Refactored the commands to rationalize arguments. Added required addi…

434f77b

…tional vpc parameters

Better messaging

eb1ef9d

Removed wait for task to stop function as we already check for a spec…

18ef5b0

…ific log to be displayed

Can now run the commands from the non-prod AWS profile

a67dcbb

Refactored the start task function into the DatabaseCopy class

94960d6

Add missing type hints

156cd26

Error handling for database not found

d93f946

Bad env case

6aa7cc7

Bad app case

aec6c4d

Handling bad account id errors

a22f4c4

Added in error cases for the load command

262a1ed

Inferring the account_id from the environment

295f9d4

Test refactor

70bf629

Refactored tests part 2

9e7839e

Refactored tests part 3

4f38db6

Drop --account-id cli option

05668aa

Application can now be picked up from platform-config.yml

a12ace7

Added documentation to the commands

05f5fbd

Updated log message.

15a6f96

Look up vpcs from deploy repo if possible

e5bb6f9

Make cli options consistent

ed41959

Now handling errors in the commands and aborting

a871a38

Added database-copy buildspec

7307022

antroy-madetech and others added 17 commits October 29, 2024 16:47

Move data delete to immediately after the restore

3afce56

Handle errors and clean up

ec1f095

Make error handling work (jq swallowing errors)

f230960

Added in the required CLUSTER_NAME env var for the load action

0821fe8

Changed message to a warning that the operation is destructive

8a26a9f

Wired in the online/offline functionality

610c8a5

Enriching the vpc from config in the copy command

e225c8c

Add --no-maintenance-page flag to copy command.

3be8f59

Don't show maintenance page if no_maintenance_page is True.

1384485

Black was unhappy :-/

958f1d1

Document --no-maintenance-page flag.

1aadf26

Undocument --no-maintenance-page flag.

2db56c9

Initial refactoring

162415d

Moved load balancer functions out into a provider file

7873453

Copied offline_command and related functions into domain file

b22b8d8

Refactored online/offline out of the environment command

b99dd7a

Use itertools.count to provide rule priorities.

f5bd1de

antroy-madetech force-pushed the DBTP-1109-DataCopy_2 branch from 7ad2411 to f5bd1de Compare October 29, 2024 16:48

ksugden reviewed Oct 30, 2024

View reviewed changes

antroy-madetech added 3 commits October 30, 2024 14:01

Updated docs

4d90dd5

Rename online/offline methods

a97b69b

Merge branch 'main' into DBTP-1109-DataCopy_2

9e554db

ksugden approved these changes Oct 30, 2024

View reviewed changes

antroy-madetech merged commit 410cd56 into main Oct 30, 2024
9 checks passed

antroy-madetech deleted the DBTP-1109-DataCopy_2 branch October 30, 2024 14:17

github-actions bot mentioned this pull request Oct 30, 2024

chore(main): release 11.1.0 #606

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Delete data dump from S3 after data load has been successful #600

feat: Delete data dump from S3 after data load has been successful #600

antroy-madetech commented Oct 17, 2024 •

edited

Loading

WillGibson Oct 29, 2024

antroy-madetech Oct 29, 2024

antroy-madetech Oct 29, 2024

WillGibson Oct 29, 2024

antroy-madetech Oct 29, 2024

ksugden Oct 30, 2024

ksugden Oct 30, 2024 •

edited

Loading

ksugden Oct 30, 2024 •

edited

Loading

ksugden Oct 30, 2024

antroy-madetech Oct 30, 2024

ksugden Oct 30, 2024 •

edited

Loading

feat: Delete data dump from S3 after data load has been successful #600

feat: Delete data dump from S3 after data load has been successful #600

Conversation

antroy-madetech commented Oct 17, 2024 • edited Loading

Checklist:

Title:

Description:

Tasks:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksugden Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

ksugden Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksugden Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

antroy-madetech commented Oct 17, 2024 •

edited

Loading

ksugden Oct 30, 2024 •

edited

Loading

ksugden Oct 30, 2024 •

edited

Loading

ksugden Oct 30, 2024 •

edited

Loading