Skip to content
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

Cache costexplorer API #1280

Merged
merged 15 commits into from
Jan 4, 2024
Merged

Conversation

Azanul
Copy link
Collaborator

@Azanul Azanul commented Dec 14, 2023

Problem

  • Last 6 months of cost data is not present
  • Region shouldn't be considered for filtering costexplorer API data

Solution

  • Cache the costexplorer output for last 6 months in local json file
  • Disregard region

Notes

  • Kept the region argument as it'd be useful in the near future

Checklist

  • Code follows the contributing guidelines
  • Changes have been thoroughly tested
  • Documentation has been updated, if necessary
  • Any dependencies have been added to the project, if necessary

Reviewers

@mlabouardy @jakepage91

Azanul and others added 14 commits November 28, 2023 19:17
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
@Azanul Azanul marked this pull request as ready for review December 14, 2023 11:07
@Azanul Azanul changed the title Cost explorer api Cache costexplorer API Dec 14, 2023
Copy link
Contributor

@jakepage91 jakepage91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@AvineshTripathi AvineshTripathi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep the cache file as temp file with a scope of application life cycle

@Azanul
Copy link
Collaborator Author

Azanul commented Dec 17, 2023

I would prefer to keep the cache file as temp file with a scope of application life cycle

we want to keep the file even when application turns off or crashes. Since it charges user to make requests, we'll have the user to set frequency for this separately having 1 week by default.

@AvineshTripathi
Copy link
Collaborator

we might wanna change the location of the file and not keep it in current dir

@Azanul
Copy link
Collaborator Author

Azanul commented Dec 17, 2023

we might wanna change the location of the file and not keep it in current dir

open to suggestions

@AvineshTripathi
Copy link
Collaborator

I would keep it in /tmp dir maybe

@Azanul
Copy link
Collaborator Author

Azanul commented Dec 25, 2023

I would keep it in /tmp dir maybe

Putting it in /tmp would clear it on device/container reboot

@AvineshTripathi
Copy link
Collaborator

@Azanul for now we can keep it in same place, please update the branch I ll give approve

@mlabouardy mlabouardy merged commit 3af554b into tailwarden:develop Jan 4, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants