-
Notifications
You must be signed in to change notification settings - Fork 62
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
added more fallback tests + using a file #873
added more fallback tests + using a file #873
Conversation
xTRam1
commented
Jun 16, 2023
- Modified the tests so that they load from an actual quota file instead of me defining a dictionary.
- Modified planner so that it can accept a file name for the quota limits (default to the skyplane config quota files)
- Added more tests for error conditions (no quota file is provided + quota file is provided but the requested region is not included in the quota file)
…is actually exists
Remove duplicated planners
Instead of storing the vcpu limits as a hardcoded variable inside Planner, created a new .csv file that includes that information. When Planner is created, we read this csv file. Wrote test cases that include fake quota limits.
This allows for testing transfers locally, both for synthetic data and real object store data.
…ne-project#865) S3 currently does not have proper bucket cleanup after testing leading to `TooManyBuckets` errors on tests after a certain point. This adds the cleanup logic to every integration test. Furthermore, the hadoop jdk installs were removed as they should not be required anymore.
…kyplane-project#868) This fixes two bugs: * During chunk dispatching for CLI transfers, Ctrl-C could not be used to exit out of the transfer due to threads not being properly filled. This is fixed by adding exit signals to the tracker and stopped the multipart threads when the tracker is exited. * Previous `self.auth` in the `Server` instance was modified to contain credentials for all clouds that are initialized, however this was not properly updated for child classes.
if os.path.exists(gcp_quota_path): | ||
with gcp_quota_path.open("r") as f: | ||
self.quota_limits["gcp"] = json.load(f) | ||
quota_limits = {} |
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.
Planner can take custom quota limit files for better testing
@@ -121,7 +125,9 @@ def _calculate_vm_types(self, region_tag: str) -> Optional[Tuple[str, int]]: | |||
|
|||
# No quota limits (quota limits weren't initialized properly during skyplane init) | |||
if quota_limit is None: | |||
logger.warning(f"Quota limit file not found for {region_tag}") | |||
logger.warning( |
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.
Improved the warning message
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.
lgtm!
* Modified the tests so that they load from an actual quota file instead of me defining a dictionary. * Modified planner so that it can accept a file name for the quota limits (default to the skyplane config quota files) * Added more tests for error conditions (no quota file is provided + quota file is provided but the requested region is not included in the quota file) --------- Co-authored-by: Sarah Wooders <[email protected]> Co-authored-by: Asim Biswal <[email protected]>