-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[match] add profile_type filtering when checking if profile exists #20311
[match] add profile_type filtering when checking if profile exists #20311
Conversation
86c6c90
to
fa23628
Compare
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.
Thanks for optimizing fetching profiles :) This is great.
match/lib/match/spaceship_ensure.rb
Outdated
# But we can filter provisioning profiles based on their type (this, in general way faster than getting all profiles) | ||
filter = { profileType: profile_types(type).join(",") } if type | ||
found = Spaceship::ConnectAPI::Profile.all(filter: filter).find do |profile| |
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.
This is so smart! Slightly embarrassed I didn't do this right away 🙈
d25b4c2
to
7d683b2
Compare
types = profile_types(prov_type) | ||
# Filtering on 'profileType' seems to be undocumented as of 2020-07-30 | ||
# but works on both web session and official API | ||
types = Match.profile_types(prov_type) |
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.
Same here. this information is actually mentioned in documentation at this moment (2022-05-22)
https://developer.apple.com/documentation/appstoreconnectapi/list_and_download_profiles#query-parameters
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 🔥 Thanks for improving performance :)
@joshdholtz Is there anything needed to be added? |
bump |
7d683b2
to
6a4d9a8
Compare
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.
Thanks for this PR! This is so so so much better 😊
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.
Congratulations! 🎉 This was released as part of fastlane 2.207.0 🚀
Checklist
bundle exec rspec
from the root directory to see all new and existing tests passbundle exec rubocop -a
to ensure the code style is validMotivation and Context
When
match
checking if profile exists on AppStore Connect, the check is relatively slow, if performed without filtering.App Store Connect API doesn't support filtering by the profile
uuid
, but it supports filtering byprofileType
.Adding filtering by
profileType
significantly speeds up requests, thatmatch
is performinghttps://developer.apple.com/documentation/appstoreconnectapi/list_and_download_profiles#query-parameters
It seems that response time highly depends on the amount of returned items.
It was around ~20s for 85 profiles, and ~4s for 9 profiles (with filtering).
Description
Additional filtering is added by
profileType
Testing Steps