sayanchowdhury opened a new pull-request against the project: `releng` that you are following: `` Fix the clean ami script to remove the amis based on date ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` days=10 is hardcoded here - is that right? I think it should be `days=days` ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` can we re-order these two (change perms first and then delete) and then add a nice comment about what is going on?
``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` It took me a while to figure out what is going on and I'm still not sure it's entirely right (or maybe I misunderstand).
For each AMI:
- `(2*$days < item < $days)` -> **DELETE** - `($days < item < TODAY)` -> **CHANGE PERMISSIONS**
Is that right? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Here, 10 is the number of days the AMIs will stay with the changed perms. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` This help message needs more clarity. I don't quite get exactly what value you want me to give it for days. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` we need a comment at the beginning of this if/else that explains how we are determining which AMIs are OK to delete. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` actually maybe better to just explain all of the logic before any of the if statements in this function. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` ``` REGIONS = ( 'us-east-1', 'us-west-2', 'us-west-1', 'eu-west-1', 'eu-central-1', 'ap-south-1', 'ap-southeast-1', 'ap-northeast-1', 'ap-northeast-2', 'ap-southeast-2', 'sa-east-1', )
```
Do we have more regions now? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sinnykumari commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` We have nightly AMIs in two additional regions: ca-central-1 eu-west-2
``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
can we re-order these two (change perms first and then delete) and then add a nice comment about what is going on?
Done. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sinnykumari commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Looks good to me, one minor change may required: we might want to make NIGHTLY lowercase at https://pagure.io/fork/sayanchowdhury/releng/blob/0ce51d0036c4b779e71efdc4fd... , since result of fedfind is somewhat like this for rawhide and F30 nightly:
```
compose_info = fedfind.release.get_release(cid='Fedora-Rawhide-20190213.n.0') print compose_info.type
nightly ```
Also, this won't delete any of Fedora updates and updates testing AMIs, since for both cases compose_info.label starts with Update-* ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
(2*$days < item < $days) -> DELETE ($days < item < TODAY) -> CHANGE PERMISSIONS
for delete, its $days-10 < item < TODAY-10 -> DELETE ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
Looks good to me, one minor change may required: we might want to make NIGHTLY lowercase at https://pagure.io/fork/sayanchowdhury/releng/blob/0ce51d0036c4b779e71efdc4fd... , since result of fedfind is somewhat like this for rawhide and F30 nightly:
compose_info = fedfind.release.get_release(cid='Fedora-Rawhide-20190213.n.0') print compose_info.type
nightly
Yeah, `NIGHTLY` is the variable. The value of it is `nightly`.
Also, this won't delete any of Fedora updates and updates testing AMIs, since for both cases compose_info.label starts with Update-*
Yes, This is to prevent from deleting the 2WA Host released AMIs. I could not find a way to distinguish from the other AMIs. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sinnykumari commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
Yeah, NIGHTLY is the variable. The value of it is nightly.
ah yes, missed it.
Also, this won't delete any of Fedora updates and updates testing AMIs, since for both cases compose_info.label starts with Update-*
Yes, This is to prevent from deleting the 2WA Host released AMIs. I could not find a way to distinguish from the other AMIs.
Compose ID of updates are usually Fedora-$releases-updates-$composedate and for updates-testing, it is Fedora-$releases-updates-testing-$composedate . We do 2WA release from updates compose, so maybe we can filter out at compose ID level to remove AMIs from updates-testing? @dustymabe WDYT ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sinnykumari commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` @sayanchowdhury One question for my clarity, does using topic [org.fedoraproject.prod.fedimg.image.upload]( https://pagure.io/fork/sayanchowdhury/releng/blob/a7487345d6a3aa1af50acb6dad...) gets us to fetch AMIs for all desired regions we want AMIs to make private or delete or we should move to `org.fedoraproject.prod.fedimg.image.publish` topic where we have AMI IDs for all regions available? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
Yeah, NIGHTLY is the variable. The value of it is nightly.
ah yes, missed it.
Also, this won't delete any of Fedora updates and updates testing AMIs, since for both cases compose_info.label starts with Update-* Yes, This is to prevent from deleting the 2WA Host released AMIs. I could not find a way to distinguish from the other AMIs.
Compose ID of updates are usually Fedora-$releases-updates-$composedate and for updates-testing, it is Fedora-$releases-updates-testing-$composedate . We do 2WA release from updates compose, so maybe we can filter out at compose ID level to remove AMIs from updates-testing? @dustymabe WDYT
We can do this. But still updates composes would stay. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Let's define a new variable at the top of the script (`WAIT_PERIOD=10`) ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` i'm not sure if we should use `days` here at all. we basically want any AMIs that have had launch permissions removed for more than 10 days right? I almost wonder if we should be using fedmsg at all and not pure ec2 commands for this. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` we should be able to remove updates-testing AMIs IMHO. Can we use any other identifying information?
- [updates compose metadata](https://kojipkgs.fedoraproject.org/compose/updates/Fedora-29-updates-2019022...) - [updates-testing compose metadata](https://kojipkgs.fedoraproject.org/compose/updates/Fedora-29-updates-testing...) ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` If I upload an AMI today and this gets run today does that AMI get its launch permissions removed? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` @dustymabe I fixed the issue that you pointed out. Now AMIs $((today-5)-$days) < item < $(today-5) will be deleted. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Since the AMIs will be deleted after 10 days, so the $(today-15)-days < item < today -15 ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` The benefit of using this doesn't seem to be worth it to me. We define more twice and we call it twice and all but one argument is static. Let's just call get_page() instead rather than making readers lookup what `functools.partial` does? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` add comment above this. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` why should we include AMIs that have the date in them? What does it mean if they do have the date in them? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` What does this mean? Add comment ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` these should be useful for the dry-run case, no? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Here are some slight modifications that would make this more useful I think:
```patch diff --git a/scripts/clean-amis.py b/scripts/clean-amis.py index 31d069c..b518089 100755 --- a/scripts/clean-amis.py +++ b/scripts/clean-amis.py @@ -265,8 +265,16 @@ if __name__ == '__main__': help="Delete the AMIs whose launch permissions have been removed", action="store_true", default=False) argument_parser.add_argument( - "--days", - help="Specify the number of days to run the operation", + "--howfarback", + help="How far back should we search for AMIs that need to be deleted", + type=int) + argument_parser.add_argument( + "--deletewaitperiod", + help="How long to wait after launch perms were removed before deleting", + type=int) + argument_parser.add_argument( + "--permswaitperiod", + help="Specify the number of days to wait before removing permissions", type=int) argument_parser.add_argument( "--change-perms", @@ -284,9 +292,22 @@ if __name__ == '__main__': if args.delete and args.change_perms: print('Both the argument delete and change permission is not allowed')
- days = 10 - if args.days: - days = args.days + # Default to waiting 10 days after launch permissions have been + # removed before deleting + deletewaitperiod = 10 + if args.deletewaitperiod: + deletewaitperiod + + permswaitperiod = 10 + if args.permswaitperiod: + permswaitperiod = args.permswaitperiod + + # Ideally we could search through all the AMIs that ever were + # created but this would create quite the load on datagrepper. + # default to 4 weeks + howfarback = 28 # 4 weeks + if args.howfarback: + howfarback = args.howfarback
# The AMIs deleted are the nightly AMIs that are uploaded via fedimg everyday. # The clean up of the AMIs happens through a cron job. @@ -296,12 +317,20 @@ if __name__ == '__main__': # - If no issues are reported in 10 days, the AMIs are deleted permanently.
if args.change_perms: - end = (datetime.now() - timedelta(days=5)).strftime('%s') - amis = _get_nightly_amis_nd(delta=86400 * days, end=int(end)) + if howfarback < permswaitperiod: + raise Exception # shouldn't happen + end = (datetime.now() - timedelta(days=permswaitperiod)).strftime('%s') + amis = _get_nightly_amis_nd(delta=86400 * howfarback, end=int(end)) change_amis_permission_nd(amis, dry_run=args.dry_run)
if args.delete: - end = (datetime.now() - timedelta(days=15)).strftime('%s') - amis = _get_nightly_amis_nd(delta=86400 * days, end=int(end)) + if howfarback < deletewaitperiod: + raise Exception # shouldn't happen + end = (datetime.now() - timedelta(days=deletewaitperiod)).strftime('%s') + amis = _get_nightly_amis_nd(delta=86400 * howfarback, end=int(end)) + # Here we need to see how long it's been since we removed the + # permissions. If it has been deletewaitperiod days then we can + # delete. If there is no way to tell this organically then we + # could add a tag to the AMI with a timestamp at the same time + # we remove the permissions. delete_amis_nd(amis, dry_run=args.dry_run) - ```
note the comment about possibly using tags set on the AMIs to determine when the launch permissions were removed. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` any updates for today for me to review? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Specify the number of days worth of AMI fedmsg information to fetch from datagrepper. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` probably need to raise an exception here? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` I don't understand the 2nd if statement here and below for deletewaitperiod. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` need description error message here ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Need to fix function description and arg descrition here ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` This is a tag key/value pair we are defining here right? I would do something like `Key=LaunchPermissionRevoked` and `Value=todaystimestamp`. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` this seems limiting - we will only delete images that were deleted on that day. We want to delete all the ones with permissions revoked *before* that day. I'm thinking we should get all images with this tag (key) and then filter the list of AMIs based on the timestamp (value). ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` "on or before that day" or "before that day" ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` This is when you want to delete same day images. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` I don't know if I support deleting same day images but I guess it makes sense for development. The user should be able to pass `--permswaitperiod 0`, right? We should not need a special if statement for this. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` "on or before that day" works for me if it does for you. but in general "off by one" shouldn't matter too much here. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
I don't know if I support deleting same day images but I guess it makes sense for development. The user should be able to pass --permswaitperiod 0, right? We should not need a special if statement for this.
since it's 0, the if condition evaluated to false and the default value of 10 is set. And yes it's needed for development purpose. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
"on or before that day" works for me if it does for you. but in general "off by one" shouldn't matter too much here.
+1 ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
I don't know if I support deleting same day images but I guess it makes sense for development. The user should be able to pass --permswaitperiod 0, right? We should not need a special if statement for this.
since it's 0, the if condition evaluated to false and the default value of 10 is set. And yes it's needed for development purpose.
I see. In that case we should change things up a bit. Let's set the defaults for these values in argparse calls above and then just use `args.days` `args.permswaitperiod` and `args.deletewaitperiod` everywhere - WDYT? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
I see. In that case we should change things up a bit. Let's set the defaults for these values in argparse calls above and then just use args.days args.permswaitperiod and args.deletewaitperiod everywhere - WDYT?
Sounds good to me ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` just checking - not sure what `ami['Tags']` returns but i would think this should be `tag.keys()` and not tag.values.. Do you have an example of what `tags=ami['Tags']` returns? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` i would make the function args here generic.. ie timestamp1, timestamp2 and add a description: `eturns true if timestamp1 is greater than or equal to timestamp2` ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` maybe rename the function to `_timestamp_greater()` the readability of the later code benefits from this ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` this shouldn't happen. I would at least print out a warning message with information ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
just checking - not sure what ami['Tags'] returns but i would think this should be tag.keys() and not tag.values.. Do you have an example of what tags=ami['Tags'] returns?
[{'Key': 'LaunchPermissionRevoked', 'Value': '27022019'}]
List of dictionaries ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` add a comment before this if statement - something like ``` # The revoke timestamp allows us to tell how long ago an image had permissions removed # If the permissions have been removed for shorter than the waiting period then we can't # delete it yet. ```
maybe we should change the function to be titled `is_timestamp_older()` ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` i may have given you a bug - not sure if the string can span two lines here. maybe test it and see. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` LGTM - other than the one comment.
@sinnykumari - can you review this in the morning? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: ``
i may have given you a bug - not sure if the string can span two lines here. maybe test it and see.
Yeah I had fixed that in the next commit. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sinnykumari commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Note that by this method we will be able to fetch TwoWeek compose id only for recent ones (from the time we added multi-arch support in twoweek.complete fedmsg topic). If we are interested to deleting most of the older composes, then we will have to add another condition to get composeID, sample json is https://apps.fedoraproject.org/datagrepper/id?id=2018-01994496-c703-49d6-8ff... ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sinnykumari commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` This idea came to my mind today as well, nice to see that this is already taken care of! ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sinnykumari commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Added one concern above, other than that LGTM. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sinnykumari commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` LGTM, have re-looked only into the modified content which I believe would be the "older-format fedmsg messages" section ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` need to set default value here ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` need to set default value here right? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` can you verify that changing this from `'-(\d{8}).'` to `"-(\d{8})."` doesn't cause any problems? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
mohanboddu commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Why is this seperated? Cant it be joined in the for loop below? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
mohanboddu commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Use elif since same arch wont be repeated and less checks :smile: ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
mohanboddu commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Same as above ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` these changes LGTM - can you address mohan's comments?
``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
mohanboddu commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` Same as the one that I asked above, why separated, rather than being the for loop below? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
mohanboddu commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` I have no idea on how delete amis and change ami permissions work, but other than that and the above comments I made, its LGTM. ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
sayanchowdhury commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` No because the first query returns the number of pages. which is utilized in the next loop ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
dustymabe commented on the pull-request: `Fix the clean ami script to remove the amis based on date` that you are following: `` LGTM. Let's squash and merge and then can you:
1. do a test against dev account 2. do a dry-run against prod account and share output with us in IRC? ``
To reply, visit the link below or just reply to this email https://pagure.io/releng/pull-request/8146
mohanboddu merged a pull-request against the project: `releng` that you are following.
Merged pull-request:
`` Fix the clean ami script to remove the amis based on date ``
rel-eng@lists.fedoraproject.org