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

API Throttling #10755

Closed
2 tasks done
palmeraeasd opened this issue Mar 1, 2022 · 17 comments
Closed
2 tasks done

API Throttling #10755

palmeraeasd opened this issue Mar 1, 2022 · 17 comments

Comments

@palmeraeasd
Copy link

Debug mode

Describe the bug

Summary:
API Throttling not behaving as intended when editing Kernel.php as described at: https://snipe-it.readme.io/reference/api-throttling

Background:
I have a BASH script that makes API requests. I had an older snipe-it install I was working with before wiping the server and starting new. I'm not sure what the old version was but there was no API throttling. The problem now is that I am making tens of thousands of requests per script run (I will do this every night) and it went from taking an hour to run to several hours with API throttling. I tried changing the API Throttle rate in Kernel.php to shorten the run time but it had the opposite effect.

Exact Issue:
When changing the line in Kernel.php from 120 to 500 requests per minute, the true number of requests allowed goes down to 60.

Additional Question:
If editing Kernel.php DID work, can I just set it to 99999999 so there is effectively no throttle rate or will this have bad consequences?

Reproduction steps

Run a script making lots of API requests and log the number of requests when hitting a throttle.

Expected behavior

When changing Kernel.php to allow 500 requests instead of 120, it should allow 500 but is instead allowing only 60.

Screenshots

No response

Snipe-IT Version

v5.4.0 - build 6685

Operating System

Ubuntu 20.04 Server

Web Server

Apache

PHP Version

7.4.3

Operating System

Ubuntu 18.04

Browser

Command Line (BASH)

Version

No response

Device

No response

Operating System

No response

Browser

No response

Version

No response

Error messages

No response

Additional context

Installed via install.sh

@snipe
Copy link
Owner

snipe commented Mar 1, 2022

120 requests per second is reasonable for most reasonable automations. But hey, it’s your server, so you can do whatever you like.

Per the release notes, there is a new env var in v5.4.0 that you will want to set to override the settings: https://github.com/snipe/snipe-it/releases/tag/v5.4.0

GitHub
⚠️ IMPORTANT: Later versions of Snipe-IT will require PHP 7.4 or greater. It is highly recommended you upgrade your version of PHP NOW. (This is a requirement in order for us to be able to pull fo...

@snipe snipe closed this as completed Mar 1, 2022
@palmeraeasd
Copy link
Author

Thank you for the info and thank you for creating this great software. I was just asking your opinion if you think upping the throttle rate that much will cause any errors.

I feel like I need to explain why I am making so many API requests.

We are a public school district with over 10,000 students and are 1-to-1 with iPads. I need to sync data from JAMF, our mobile device management system because there can be dozens of ownership changes per day on the iPads. Doing this means verifying the user exists in the inventory and information is up to date (1 request to get the user info, then possibly 1 request to patch/update) and the same for the iPad. This means one script run can generate up to 4 API requests per asset which would be 40,000 API requests for only our iPads (we also have 1k+ laptops for staff). I'd like to run this overnight as to not tax the server during the daytime when we are using the GUI.

Can I do this programmatically with a CSV import instead of individual API requests?

I hope that makes sense.

Thank you

@uberbrady
Copy link
Collaborator

There is a jamf2snipe script that does a lot of that - but that can definitely start to hit throttles as well, and I think it basically works that way (presuming that you aren’t already using that).

I don’t think you will blow yourself up too badly by raising the throttle rate, though - if you do, then lower it a little.

@snipe
Copy link
Owner

snipe commented Mar 1, 2022

@palmeraeasd - So I’m clear, each user could be running:

  1. Get user
  2. Possibly update user
  3. Get iPad
  4. Possibly update iPad

Do I have that right? Which endpoint are you using for the user fetch?

Also, as @uberbrady mentioned, we do have lots of folks who use Jamf2Snipe, and IIRC, that has a built-in sleep between requests.

@snipe
Copy link
Owner

snipe commented Mar 1, 2022

(What specific user info would normally be updated in one of possible update queries? And does your script check to see if the update is actually needed or if the info is the same and the update can be skipped?)

@palmeraeasd
Copy link
Author

palmeraeasd commented Mar 1, 2022

There is actually a 5th step of checking if the Model exists and getting the model ID but this could be eliminated by hard coding the model ID's. I will implement this tomorrow.

I am using /api/v1/users?email= to search for the user which tells me if they exist and then using that same reply to see if an update is necessary. It only does the second request (both for user and asset) if a change is necessary. It also only does 1 request for all changes to an asset. The only real change for a user would be their name (this happens more often than you would think for a district of our size) but I am technically checking that their student ID matches as well. I figure I'm getting the user info by checking if they exist anyways so I might as well verify everything matches.

I built in error-handling for API throttling where when it hits the limit, it sleeps for 60 seconds before retrying and continuing. The problem is how long it takes to run the script doing this. I could also just run a sleep .5 seconds after each request but if someone is using the GUI it counts as an API request so I would still have to detect and delay more.

@snipe
Copy link
Owner

snipe commented Mar 1, 2022

IIRC, if someone is using the GUI, it would only count if the user the API queries run as is logged into the GUI. Some folks get around this by having a dedicated user+token just for API stuff.

What would the changes be that happen to the asset (iPad)? I’m not trying to interrogate you, of course, just trying to understand the workflow you’ve got going on, so I can either recommend a more streamlined approach, or potentially add some additional endpoints that might help. For example, we just added a parameter for the user’s endpoint that lets you load their consumables along with their basic data, if you pass that along to the endpoint query. We could do something similar for assets, etc, which could mean that we eliminate one of those queries.

(The model would always exist on an asset, so you probably don’t need to check there. It will either be a real model, or something like “model not found”.)

@palmeraeasd
Copy link
Author

That makes sense about the GUI. I am running the API key under a dedicated admin account and I will be enabling Google SAML for our techs so that shouldn't be an issue then.

As for the asset, JAMF doesn't populate a lot of fields until the iPad is enrolled so when that happens I will sync the model name, assigned user, wifi MAC address, if lost mode is enabled, the iOS version, and notes. We are about to get 6,000 new iPads which I will import via CSV with only asset tag and serial number then the other info will be pulled from JAMF when the iPad is enrolled. For current assets in JAMF I am creating them with the script.

The important things that change often are assigned user, if lost mode is enabled, and notes

We constantly have new students, leaving students, and broken iPad exchanges so these things change frequently.

Thank you both.

@snipe
Copy link
Owner

snipe commented Mar 1, 2022

You’ll need to have the model name in the CSV in order to import them though - you can’t import assets (via API or CSV) with an invalid or missing model, or it will fail at the model validation level.

If the assigned user is different, are you doing a proper checkin/checkout to the new user, or just trying to edit the record itself?

Curious if you’ve checked out Jamf2Snipe? Is there something you need that it doesn’t do? We will likely. Be getting much more directly involved with that project in the coming months, so curious to see what might be missing from that.

@palmeraeasd
Copy link
Author

Yes, I forgot to say I will know the asset model for the csv import.

I am simply setting the value of 'assigned_to' to either the user ID or null with the API request.

I didn't know Jamf2Snipe exists and at this point, I've already built my script and it does more checking on the JAMF data such as checking for duplicate asset tags (JAMF does not enforce this!), if data has a tab in it (\t will appear at the end of a username or email sometimes if someone copies and pastes the info into JAMF), sometimes asset tag is blank in JAMF, I verify the student's username and email are the same in JAMF (checking for human errors), and checking if each student is assigned more than 1 iPad.

It sends an email with all of this info and does not add/update these assets to snipe-it if there is an issue with the data.

@snipe
Copy link
Owner

snipe commented Mar 1, 2022

I’ll have to check, but if you’re just accepting a changed value of the assigned_to, it may bypass any notifications and logging that’s supposed to be used when an asset changes hands. I’ll look into the code and confirm.

@palmeraeasd
Copy link
Author

palmeraeasd commented Mar 1, 2022

I just noticed I am using assigned_user during creation but during update, I am using assigned_to

could this be the reason for my other thread's issue? (#10702)

I should probably change that to assigned_user always? I think the API returns assigned_to in the asset info and that's why I was thinking to use that.

I read on the docs (https://snipe-it.readme.io/reference/hardware-create)
"You can do a checkout on creation if you add one of the following fields: assigned_user, assigned_asset, or assigned_location. This should be a valid primary key of the user, asset or location you wish to checkout to."

so I assumed I can just use that to assign and unassign.

Snipe-IT Documentation
Snipe-IT is a free, open source IT asset management system. Features include management of assets, users, licenses, accessories, consumables and components, as well as two-factor authentication, LDAP/AD syncing, and asset acceptance confirmation.

@palmeraeasd
Copy link
Author

palmeraeasd commented Mar 1, 2022

https://snipe-it.readme.io/reference/hardware-partial-update

I am using patch which lists:
assigned_to
int32
The id of the user the asset is currently checked out to

Snipe-IT Documentation
Snipe-IT is a free, open source IT asset management system. Features include management of assets, users, licenses, accessories, consumables and components, as well as two-factor authentication, LDAP/AD syncing, and asset acceptance confirmation.

@palmeraeasd
Copy link
Author

The first run yesterday which had to create all the users and assets took over 10 hours to run. Ran it again changing the API limit to 600 and it was completed in under an hour with no timeouts! I think the main thing was the fact it had to create all the accounts and assets during the first run.

Thanks for all of your help. And please let me know if I should not be using assigned_to to assign and unassign assets.

@palmeraeasd
Copy link
Author

@snipe Update: I have encountered the same issue as #10702 so it must be a problem with using assigned_to when patching

It sounds like I need to change my script to use /hardware/:id/checkin and /hardware/:id/checkout which will add two more API requests for changing the user of an assigned asset.

For the affected assets I am unable to check in or out via the API.
On checkout I get: "That asset is not available for checkout!"
On checkin I get: "Server Error"

I think I'll have to delete the asset and let the script re-add it. The problem is figuring out how many assets this is affecting.

@snipe
Copy link
Owner

snipe commented Mar 2, 2022

@palmeraeasd Normally when we see that, it's because the assigned_type isn't being added and/or a checkout record isn't being created. It's trying to determine if it should join the assets table to users, license seats, etc - and if those fields are not populated, it doesn't know what to join to.

@snipe
Copy link
Owner

snipe commented Mar 2, 2022

The methods you should be using there, if the assigned_to has changed would be these:

https://snipe-it.readme.io/reference/hardware-checkin
https://snipe-it.readme.io/reference/hardware-checkout

public function checkout(AssetCheckoutRequest $request, $asset_id)
{
$this->authorize('checkout', Asset::class);
$asset = Asset::findOrFail($asset_id);
if (!$asset->availableForCheckout()) {
return response()->json(Helper::formatStandardApiResponse('error', ['asset'=> e($asset->asset_tag)], trans('admin/hardware/message.checkout.not_available')));
}
$this->authorize('checkout', $asset);
$error_payload = [];
$error_payload['asset'] = [
'id' => $asset->id,
'asset_tag' => $asset->asset_tag,
];
// This item is checked out to a location
if (request('checkout_to_type')=='location') {
$target = Location::find(request('assigned_location'));
$asset->location_id = ($target) ? $target->id : '';
$error_payload['target_id'] = $request->input('assigned_location');
$error_payload['target_type'] = 'location';
} elseif (request('checkout_to_type')=='asset') {
$target = Asset::where('id','!=',$asset_id)->find(request('assigned_asset'));
$asset->location_id = $target->rtd_location_id;
// Override with the asset's location_id if it has one
$asset->location_id = (($target) && (isset($target->location_id))) ? $target->location_id : '';
$error_payload['target_id'] = $request->input('assigned_asset');
$error_payload['target_type'] = 'asset';
} elseif (request('checkout_to_type')=='user') {
// Fetch the target and set the asset's new location_id
$target = User::find(request('assigned_user'));
$asset->location_id = (($target) && (isset($target->location_id))) ? $target->location_id : '';
$error_payload['target_id'] = $request->input('assigned_user');
$error_payload['target_type'] = 'user';
}
if (!isset($target)) {
return response()->json(Helper::formatStandardApiResponse('error', $error_payload, 'Checkout target for asset '.e($asset->asset_tag).' is invalid - '.$error_payload['target_type'].' does not exist.'));
}
$checkout_at = request('checkout_at', date("Y-m-d H:i:s"));
$expected_checkin = request('expected_checkin', null);
$note = request('note', null);
$asset_name = request('name', null);
// Set the location ID to the RTD location id if there is one
// Wait, why are we doing this? This overrides the stuff we set further up, which makes no sense.
// TODO: Follow up here. WTF. Commented out for now.
// if ((isset($target->rtd_location_id)) && ($asset->rtd_location_id!='')) {
// $asset->location_id = $target->rtd_location_id;
// }
if ($asset->checkOut($target, Auth::user(), $checkout_at, $expected_checkin, $note, $asset_name, $asset->location_id)) {
return response()->json(Helper::formatStandardApiResponse('success', ['asset'=> e($asset->asset_tag)], trans('admin/hardware/message.checkout.success')));
}
return response()->json(Helper::formatStandardApiResponse('error', ['asset'=> e($asset->asset_tag)], trans('admin/hardware/message.checkout.error')));
}

Snipe-IT Documentation
Snipe-IT is a free, open source IT asset management system. Features include management of assets, users, licenses, accessories, consumables and components, as well as two-factor authentication, LDAP/AD syncing, and asset acceptance confirmation.
Snipe-IT Documentation
Snipe-IT is a free, open source IT asset management system. Features include management of assets, users, licenses, accessories, consumables and components, as well as two-factor authentication, LDAP/AD syncing, and asset acceptance confirmation.

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

No branches or pull requests

3 participants