Feature: Backup api #7231

archinksagar · 2024-01-19T16:52:59Z

Please provide feedback on this initial draft of Backup API.
I have added the following API endpoints.

create_backup_plan
create_backup_vault
get_backup_plan - in progress
describe_backup_vault - in progress
delete_backup_plan
list_backup_plans
list_backup_vaults - in progress
list_tags
tag_resource
untag_resource

Things yet to be implemented

describe_backup_vault
Deletion_date and LastExecutionDate in get_backup_plan, list_backup_plans
NumberOfRecoveryPoints, MinRetentionDays, MaxRetentionDays, LockDate in list_backup_vaults
Pagination
Exceptions

bblommers

Hi @archinksagar, thanks for the PR! This looks good overall, I just have a few nitpicks.

Note that the linter is currently failing as well: https://github.com/getmoto/moto/actions/runs/7586740652/job/20665598379?pr=7231

bblommers · 2024-01-19T20:09:21Z

tests/test_backup/test_server.py

@@ -0,0 +1,13 @@
+"""Test different server responses."""


This file doesn't seem to add anything, so let's just delete it

bblommers · 2024-01-19T20:10:20Z

moto/__init__.py

+mock_backup = lazy_load(".backup", "mock_backup", boto3_name="backup")
+mock_backup = lazy_load(".backup", "mock_backup", boto3_name="backup")
+mock_backup = lazy_load(".backup", "mock_backup", boto3_name="backup")
+mock_backup = lazy_load(".backup", "mock_backup", boto3_name="backup")


This needs to only be added once (I know it's a bug in the scaffold-script, it adds this line without checking)

bblommers · 2024-01-19T20:11:33Z

moto/backup/models.py

+    def random_str_utf8(self) -> str:
+        ran_str = mock_random.get_random_string(length=48)
+        utf8_bytes = ran_str.encode('utf-8')
+        return utf8_bytes.decode('utf-8')


What is the reasoning behind the decode/encode operation? It looks like we can just return mock_random.get_random_string(length=48) to get the same outcome. (Or even get rid of the function altogether..)

bblommers · 2024-01-19T20:17:22Z

Things yet to be implemented

Out of interest - do you want to add them to this PR as well? Part of a later PR? Or is it just a general note that they are for someone else to implement?

All options are fine by me - I'm just trying to get a feel for how 'ready' this PR is 🙂

archinksagar · 2024-01-19T21:00:24Z

@bblommers Thanks for your feedback. Except for pagination and describe_backup_vault , I will implement Exceptions and others I listed. Apart from that, I realized i have to modify "resourcegroupstaggingapi" to use vault tags. I am having trouble figuring out the values for these fields, I don't know when and how they get populated- "Deletion_date" and "LastExecutionDate", "NumberOfRecoveryPoints". Is there any documentation I can refer?

bblommers · 2024-01-19T21:17:19Z

If I read the docs here correctly:

DeletionDate would only be set after delete_backup_plan is called
LastExecutionDate can be empty (None), and maybe should be - as there is no option yet to start_restore_job

The term RecoveryPoints is the same as Backup - so NumberOfRecoveryPoints is just the number of backups that single vault contains. I guess that would be always 0 for now, until we implement start_backup_job

archinksagar · 2024-01-25T19:17:39Z

hi @bblommers I have added code to resourcegrouptaggingapi and made few other modifications. I did run make lint, and fixed linting errors. Not sure why the linting is failing.

bblommers · 2024-01-25T19:40:00Z

Hey @archinksagar, which version of black you do have installed? Different versions will produce different results. Moto uses black==22.3.0

Running black moto tests should fix that (step of the) build.

https://github.com/getmoto/moto/actions/runs/7659295607/job/20874125053?pr=7231

bblommers · 2024-01-25T19:57:03Z

moto/backup/models.py

+        self.deletion_date: Optional[float] = None
+        # Deletion Date is updated when the backup_plan is deleted
+        self.last_execution_date = None  # start_restore_job not yet supported
+        rules = backup_plan.get("Rules")


Suggested change

rules = backup_plan.get("Rules")

rules = backup_plan["Rules"]

The Rules argument is mandatory anyway. This should fix the mypy error (moto/backup/models.py:34:21: error: Item "None" of "Any | None" has no attribute "__iter__" (not iterable) [union-attr])

archinksagar · 2024-01-25T20:46:51Z

@bblommers Its failing at test/test with "def delete_backup_plan(self, backup_plan_id: str) -> tuple[str, str, float, str]:
E TypeError: 'type' object is not subscriptable" error.
How do i test it in my local, is it 'make test-only'? That would take a long time. Is there a command to test only backup?

bblommers · 2024-01-25T20:52:17Z

@bblommers Its failing at test/test with "def delete_backup_plan(self, backup_plan_id: str) -> tuple[str, str, float, str]: E TypeError: 'type' object is not subscriptable" error. How do i test it in my local, is it 'make test-only'? That would take a long time. Is there a command to test only backup?

@archinksagar You could just run pytest -sv tests/test_backup to only get the tests in that directory.
I think the problem is that tuple is only available as of Python 3.9, that's why 3.7 and 3.8 fail to compile but everything else does run. So if you only test it in a recent Python version, the problem would not show up.

You can use from typing import Tuple instead (note the upper-case T) - that will work in every Python version.

bblommers · 2024-01-25T22:31:15Z

@archinksagar Hope you don't mind - I just pushed a fix that should get the ServerMode tests to pass.

The initial version intercepted all URL's (/.+), which is fine for our own request matcher. But when running the tests in ServerMode, the request matching is done by the werkzeug-module, and they require the URL's to be spelled out.

codecov · 2024-01-25T23:25:21Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (99e01b5) 95.90% compared to head (5d57a58) 95.91%.
Report is 24 commits behind head on master.

Files	Patch %	Lines
moto/backup/models.py	98.37%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #7231    +/-   ##
========================================
  Coverage   95.90%   95.91%            
========================================
  Files         840      845     +5     
  Lines       82608    82958   +350     
========================================
+ Hits        79226    79569   +343     
- Misses       3382     3389     +7

Flag	Coverage Δ
servertests	`35.44% <23.44%> (-0.06%)`	⬇️
unittests	`95.85% <99.04%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

archinksagar · 2024-01-26T16:49:05Z

@archinksagar Hope you don't mind - I just pushed a fix that should get the ServerMode tests to pass.

The initial version intercepted all URL's (/.+), which is fine for our own request matcher. But when running the tests in ServerMode, the request matching is done by the werkzeug-module, and they require the URL's to be spelled out.

@bblommers Thanks for fixing it.

bblommers

LGTM - thank you so much for adding this feature to Moto @archinksagar!

moto-payments-app · 2024-01-26T21:52:46Z

Hi @archinksagar,

Thank you again for contributing to Moto, because PR's like this are the big reason that Moto is as successful as it is.

To show our thanks, we'd like to share some of the donations that we've received with you - it's only fair that you, as a contributor, get to share the spoils.

We've created a companion website with more information:
https://payments.getmoto.org/

Feel free to open a bug or discussion if you run into any problems:
https://github.com/getmoto/payments

github-actions · 2024-01-26T23:24:30Z

This is now part of moto >= 4.2.14.dev30

New feature Backup api

1d1c6c2

bblommers requested changes Jan 19, 2024

View reviewed changes

archinksagar added 2 commits January 24, 2024 10:07

feat: Added resourcetaggingapi

a4087c5

feat: fixed lint errors

2ea349c

feat: fix linting errors -2

6d9d0f9

bblommers reviewed Jan 25, 2024

View reviewed changes

feat: fixed mypy error

8281455

archinksagar and others added 2 commits January 25, 2024 15:54

feat: fix tuple error

53ebbc4

Backup: List intercepted URL's explicitly

5d57a58

feat: Added 2 test cases

d56f2e0

archinksagar requested a review from bblommers January 26, 2024 18:00

bblommers approved these changes Jan 26, 2024

View reviewed changes

bblommers changed the title ~~New feature Backup api~~ Feature: Backup api Jan 26, 2024

bblommers added this to the 4.2.14 milestone Jan 26, 2024

bblommers linked an issue Jan 26, 2024 that may be closed by this pull request

Question about plans on implementing AWS Backups #5547

Closed

bblommers merged commit 6c7dff6 into getmoto:master Jan 26, 2024
35 checks passed

moto-payments-app bot added the payment-approved label Jan 26, 2024

archinksagar deleted the feature/backupapi branch November 23, 2024 21:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Backup api #7231

Feature: Backup api #7231

archinksagar commented Jan 19, 2024

bblommers left a comment

bblommers Jan 19, 2024

bblommers Jan 19, 2024

bblommers Jan 19, 2024

bblommers commented Jan 19, 2024

archinksagar commented Jan 19, 2024 •

edited

Loading

bblommers commented Jan 19, 2024

archinksagar commented Jan 25, 2024 •

edited

Loading

bblommers commented Jan 25, 2024

bblommers Jan 25, 2024

archinksagar commented Jan 25, 2024

bblommers commented Jan 25, 2024 •

edited

Loading

bblommers commented Jan 25, 2024

codecov bot commented Jan 25, 2024

archinksagar commented Jan 26, 2024

bblommers left a comment

moto-payments-app bot commented Jan 26, 2024

github-actions bot commented Jan 26, 2024

	rules = backup_plan.get("Rules")
	rules = backup_plan["Rules"]

Feature: Backup api #7231

Feature: Backup api #7231

Conversation

archinksagar commented Jan 19, 2024

bblommers left a comment

Choose a reason for hiding this comment

bblommers Jan 19, 2024

Choose a reason for hiding this comment

bblommers Jan 19, 2024

Choose a reason for hiding this comment

bblommers Jan 19, 2024

Choose a reason for hiding this comment

bblommers commented Jan 19, 2024

archinksagar commented Jan 19, 2024 • edited Loading

bblommers commented Jan 19, 2024

archinksagar commented Jan 25, 2024 • edited Loading

bblommers commented Jan 25, 2024

bblommers Jan 25, 2024

Choose a reason for hiding this comment

archinksagar commented Jan 25, 2024

bblommers commented Jan 25, 2024 • edited Loading

bblommers commented Jan 25, 2024

codecov bot commented Jan 25, 2024

Codecov Report

archinksagar commented Jan 26, 2024

bblommers left a comment

Choose a reason for hiding this comment

moto-payments-app bot commented Jan 26, 2024

github-actions bot commented Jan 26, 2024

archinksagar commented Jan 19, 2024 •

edited

Loading

archinksagar commented Jan 25, 2024 •

edited

Loading

bblommers commented Jan 25, 2024 •

edited

Loading