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

[action][download_dsyms] fix: download_dsyms with wait_for_dsym_processing is not checking the latest data from Connect API #19523

Merged
merged 4 commits into from
Dec 6, 2021

Conversation

bguidolim
Copy link
Contributor

@bguidolim bguidolim commented Oct 25, 2021

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

PR #19508 introduced download_dsyms to the Appstore Connect API, however when the symbols are still in the processing phase and we call the action with the parameter wait_for_dsym_processing set to true, the action was not updating the build data in order to get the newly created download URL.

Fix #19569

Description

A change was made to check if the number of available URLs is the same as build bundles (app or app clips) that include symbols. If these numbers are different and the flag wait_for_dsym_processing is true, so it waits 30 seconds and requests new data from the API.
The previous behavior is still there of already processed symbols.

Testing Steps

To test this you should use a combination of upload_to_testflight + download_dsyms(wait_for_dsym_processing: true) in order to make it work.

@google-cla google-cla bot added the cla: yes label Oct 25, 2021
@bguidolim
Copy link
Contributor Author

bguidolim commented Oct 25, 2021

@joshdholtz you might want to take a look at this one.

@joshdholtz
Copy link
Member

Thank you!! Looking at now 🙂

@google-cla
Copy link

google-cla bot commented Oct 25, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Oct 25, 2021
@joshdholtz joshdholtz changed the title Fix: download_dsyms with wait_for_dsym_processing is not checking the latest data from Connect API [action][download_dsyms] fix: download_dsyms with wait_for_dsym_processing is not checking the latest data from Connect API Oct 25, 2021
@joshdholtz
Copy link
Member

@googlebot I consent.

@google-cla google-cla bot added cla: yes and removed cla: no labels Oct 25, 2021
@joshdholtz
Copy link
Member

@bguidolim Thank you for adding this! Thank you for fixing this 😊 I also added some unit tests if you don't mind taking a quick look at this 😇

@bguidolim
Copy link
Contributor Author

@joshdholtz LGTM! Thanks :)

Copy link
Contributor

@ainame ainame left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you for fixing this 👍
I pointed minor improvements to make the change even better! Looking forward to seeing this in next version 😍

fastlane/lib/fastlane/actions/download_dsyms.rb Outdated Show resolved Hide resolved
fastlane/lib/fastlane/actions/download_dsyms.rb Outdated Show resolved Hide resolved
@aethe aethe mentioned this pull request Nov 3, 2021
4 tasks
@grEvenX
Copy link

grEvenX commented Nov 4, 2021

Any plans on when this will get into a release? The change to use the Apple API key for download dsyms action is really welcome, but without this change I don't think we can upgrade to that method since it won't find the dSYM files.

@mcstoufer
Copy link

This is blocking our CI flows as well now. Getting the API Key in place consolidated a lot of the meta around our project and it would great to get this last bit working.

@bguidolim
Copy link
Contributor Author

@joshdholtz @ainame ping

@grEvenX
Copy link

grEvenX commented Nov 18, 2021

@bguidolim I'm not very familiar in the ruby space and a bit off-topic, but do you know if there is any way that we can install a version of fastlane with these changes before they are released and published so we can start using it on our CIs?

bguidolim and others added 4 commits November 18, 2021 11:55
@bguidolim
Copy link
Contributor Author

@grEvenX you can use this on your Gemfile:

gem 'fastlane', github: 'bguidolim/fastlane', branch: 'fix-download-dsyms-processing'

@grEvenX
Copy link

grEvenX commented Nov 18, 2021

@bguidolim Thanks, tested and it works perfectly with these changes 👍

@mcstoufer
Copy link

Does the Fastlane team have any feeling for when this would appear in a release?

@christianYoopies
Copy link

@bguidolim @ainame any ETA of this fix in public release?

@bguidolim
Copy link
Contributor Author

@christianYoopies it's all set from my side.

@Kaspik
Copy link
Contributor

Kaspik commented Nov 29, 2021

@joshdholtz Please, can we merge this in and release a new fastlane version? 🙏🏼 Thanks!

@bguidolim
Copy link
Contributor Author

My Xmas gift would be this PR merged. 🎅🏻

@joshdholtz
Copy link
Member

Ah sorry! November was a 🎢 for me so getting things caught back up 😊 Looking again and then I'll get a new version out!

Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

This is 🔥 Thank you so much for fixing this and I'll get this released ASAP!

@joshdholtz joshdholtz merged commit 2668eff into fastlane:master Dec 6, 2021
Copy link

@fastlane-bot fastlane-bot left a 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.199.0 🚀

@bguidolim bguidolim deleted the fix-download-dsyms-processing branch December 10, 2021 20:29
@fastlane fastlane locked and limited conversation to collaborators Feb 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

download_dsyms times out
8 participants