217924 2020-10-19 15:22:37 -0700 [results.webkit.org] Refresh S3 context when token expires 2020-10-19 17:29:54 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jbedard jbedard aakash_jain dewei_zhu ryanhaddad slewis webkit-bug-importer oldest_to_newest 1699484 0 jbedard 2020-10-19 15:22:37 -0700 S3 access tokens may expire, we need to refresh the S3 context when we detect that the authentication provider has changed the access token. 1699485 1 jbedard 2020-10-19 15:23:11 -0700 <rdar://problem/70387457> 1699486 2 411806 jbedard 2020-10-19 15:30:17 -0700 Created attachment 411806 Patch 1699493 3 411806 dewei_zhu 2020-10-19 15:41:34 -0700 Comment on attachment 411806 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411806&action=review > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/model/s3_archiver.py:92 > + if self._cached_token != self.credentials.aws_session_token: Maybe adding a null check here or assert on constructor to make sure self.credentials is not None? 1699497 4 411806 jbedard 2020-10-19 15:52:28 -0700 Comment on attachment 411806 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411806&action=review >> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/model/s3_archiver.py:92 >> + if self._cached_token != self.credentials.aws_session_token: > > Maybe adding a null check here or assert on constructor to make sure self.credentials is not None? I can put this check in the __init__, because (although the review hides it), line 63 will raise an exception first. 1699510 5 411811 jbedard 2020-10-19 16:33:32 -0700 Created attachment 411811 Patch 1699514 6 411806 dewei_zhu 2020-10-19 16:42:27 -0700 Comment on attachment 411806 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411806&action=review >>> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/model/s3_archiver.py:92 >>> + if self._cached_token != self.credentials.aws_session_token: >> >> Maybe adding a null check here or assert on constructor to make sure self.credentials is not None? > > I can put this check in the __init__, because (although the review hides it), line 63 will raise an exception first. Got it. 1699515 7 dewei_zhu 2020-10-19 16:42:56 -0700 r=me 1699543 8 ews-feeder 2020-10-19 17:29:53 -0700 Committed r268702: <https://trac.webkit.org/changeset/268702> All reviewed patches have been landed. Closing bug and clearing flags on attachment 411811. 411806 2020-10-19 15:30:17 -0700 2020-10-19 16:42:27 -0700 Patch bug-217924-20201019153016.patch text/plain 2153 jbedard SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI2ODY5NSkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE2IEBACisyMDIwLTEwLTE5ICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNv bT4KKworICAgICAgICBbcmVzdWx0cy53ZWJraXQub3JnXSBSZWZyZXNoIFMzIGNvbnRleHQgd2hl biB0b2tlbiBleHBpcmVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0yMTc5MjQKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzcwMzgxMTM3PgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogU2NyaXB0cy9saWJy YXJpZXMvcmVzdWx0c2RicHkvcmVzdWx0c2RicHkvbW9kZWwvczNfYXJjaGl2ZXIucHk6CisgICAg ICAgIChTM0FyY2hpdmVyLl9faW5pdF9fKTogQWRkIF9jYWNoZWRfdG9rZW4gdmFyaWFibGUuCisg ICAgICAgIChTM0FyY2hpdmVyLl9fZW50ZXJfXyk6IFJlZnJlc2ggUzMgcmVzb3VyY2Ugd2hlbiB0 aGUgY3VycmVudCB0b2tlbiBhbmQgY2FjaGVkIHRva2VuIGRvbid0IG1hdGNoLgorICAgICAgICAo UzNBcmNoaXZlci5fX2V4aXRfXyk6IENsZWFyIG91dCB0aGUgY2FjaGVkIHRva2VuLgorCiAyMDIw LTEwLTE5ICBSeWFuIEhhZGRhZCAgPHJ5YW5oYWRkYWRAYXBwbGUuY29tPgogCiAgICAgICAgIFVu cmV2aWV3ZWQsIHJldmVydGluZyByMjY4NDk1LCByMjY4NTcxLCBhbmQgcjI2ODY4Mi4KSW5kZXg6 IFRvb2xzL1NjcmlwdHMvbGlicmFyaWVzL3Jlc3VsdHNkYnB5L3Jlc3VsdHNkYnB5L21vZGVsL3Mz X2FyY2hpdmVyLnB5Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFRvb2xzL1NjcmlwdHMvbGlicmFyaWVzL3Jlc3Vs dHNkYnB5L3Jlc3VsdHNkYnB5L21vZGVsL3MzX2FyY2hpdmVyLnB5CShyZXZpc2lvbiAyNjg2OTEp CisrKyBUb29scy9TY3JpcHRzL2xpYnJhcmllcy9yZXN1bHRzZGJweS9yZXN1bHRzZGJweS9tb2Rl bC9zM19hcmNoaXZlci5weQkod29ya2luZyBjb3B5KQpAQCAtNTQsNiArNTQsNyBAQCBjbGFzcyBT M0FyY2hpdmVyKEFyY2hpdmVyKToKICAgICAgICAgc2VsZi5yZXNvdXJjZXMgPSBOb25lCiAgICAg ICAgIHNlbGYuYnVja2V0ID0gYnVja2V0CiAgICAgICAgIHNlbGYuX2NvdW50ID0gMAorICAgICAg ICBzZWxmLl9jYWNoZWRfdG9rZW4gPSBOb25lCiAKICAgICAgICAgIyBPbmx5IG1vZGlmeSB0aGUg bGlmZWN5Y2xlIG9mIHRoZSBidWNrZXQgaWYgd2UncmUgZWRpdGluZyB0aGUgc2NoZW1hCiAgICAg ICAgIGlmIG9zLmVudmlyb25bJ0NRTEVOR19BTExPV19TQ0hFTUFfTUFOQUdFTUVOVCddID09ICcx JzoKQEAgLTg4LDcgKzg5LDggQEAgY2xhc3MgUzNBcmNoaXZlcihBcmNoaXZlcik6CiAKICAgICBk ZWYgX19lbnRlcl9fKHNlbGYpOgogICAgICAgICBzZWxmLl9jb3VudCArPSAxCi0gICAgICAgIGlm IG5vdCBzZWxmLnJlc291cmNlczoKKyAgICAgICAgaWYgc2VsZi5fY2FjaGVkX3Rva2VuICE9IHNl bGYuY3JlZGVudGlhbHMuYXdzX3Nlc3Npb25fdG9rZW46CisgICAgICAgICAgICBzZWxmLl9jYWNo ZWRfdG9rZW4gPSBzZWxmLmNyZWRlbnRpYWxzLmF3c19zZXNzaW9uX3Rva2VuCiAgICAgICAgICAg ICBzZWxmLnJlc291cmNlcyA9IGJvdG8zLnJlc291cmNlKAogICAgICAgICAgICAgICAgIHNlcnZp Y2VfbmFtZT0nczMnLAogICAgICAgICAgICAgICAgIHJlZ2lvbl9uYW1lPXNlbGYuY3JlZGVudGlh bHMucmVnaW9uX25hbWUsCkBAIC0xMDEsNiArMTAzLDcgQEAgY2xhc3MgUzNBcmNoaXZlcihBcmNo aXZlcik6CiAgICAgICAgIHNlbGYuX2NvdW50IC09IDEKICAgICAgICAgaWYgc2VsZi5fY291bnQg PD0gMDoKICAgICAgICAgICAgIHNlbGYucmVzb3VyY2VzID0gTm9uZQorICAgICAgICAgICAgc2Vs Zi5fY2FjaGVkX3Rva2VuID0gTm9uZQogICAgICAgICAgICAgc2VsZi5fY291bnQgPSAwCiAKICAg ICBkZWYgc2F2ZShzZWxmLCBhcmNoaXZlLCByZXRhaW5fZm9yPU5vbmUpOgo= 411811 2020-10-19 16:33:32 -0700 2020-10-19 17:29:54 -0700 Patch bug-217924-20201019163331.patch text/plain 2248 jbedard SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI2ODY5NSkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE2IEBACisyMDIwLTEwLTE5ICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNv bT4KKworICAgICAgICBbcmVzdWx0cy53ZWJraXQub3JnXSBSZWZyZXNoIFMzIGNvbnRleHQgd2hl biB0b2tlbiBleHBpcmVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0yMTc5MjQKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzcwMzgxMTM3PgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogU2NyaXB0cy9saWJy YXJpZXMvcmVzdWx0c2RicHkvcmVzdWx0c2RicHkvbW9kZWwvczNfYXJjaGl2ZXIucHk6CisgICAg ICAgIChTM0FyY2hpdmVyLl9faW5pdF9fKTogQWRkIF9jYWNoZWRfdG9rZW4gdmFyaWFibGUuCisg ICAgICAgIChTM0FyY2hpdmVyLl9fZW50ZXJfXyk6IFJlZnJlc2ggUzMgcmVzb3VyY2Ugd2hlbiB0 aGUgY3VycmVudCB0b2tlbiBhbmQgY2FjaGVkIHRva2VuIGRvbid0IG1hdGNoLgorICAgICAgICAo UzNBcmNoaXZlci5fX2V4aXRfXyk6IENsZWFyIG91dCB0aGUgY2FjaGVkIHRva2VuLgorCiAyMDIw LTEwLTE5ICBSeWFuIEhhZGRhZCAgPHJ5YW5oYWRkYWRAYXBwbGUuY29tPgogCiAgICAgICAgIFVu cmV2aWV3ZWQsIHJldmVydGluZyByMjY4NDk1LCByMjY4NTcxLCBhbmQgcjI2ODY4Mi4KSW5kZXg6 IFRvb2xzL1NjcmlwdHMvbGlicmFyaWVzL3Jlc3VsdHNkYnB5L3Jlc3VsdHNkYnB5L21vZGVsL3Mz X2FyY2hpdmVyLnB5Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFRvb2xzL1NjcmlwdHMvbGlicmFyaWVzL3Jlc3Vs dHNkYnB5L3Jlc3VsdHNkYnB5L21vZGVsL3MzX2FyY2hpdmVyLnB5CShyZXZpc2lvbiAyNjg2OTUp CisrKyBUb29scy9TY3JpcHRzL2xpYnJhcmllcy9yZXN1bHRzZGJweS9yZXN1bHRzZGJweS9tb2Rl bC9zM19hcmNoaXZlci5weQkod29ya2luZyBjb3B5KQpAQCAtNTQsNiArNTQsMTAgQEAgY2xhc3Mg UzNBcmNoaXZlcihBcmNoaXZlcik6CiAgICAgICAgIHNlbGYucmVzb3VyY2VzID0gTm9uZQogICAg ICAgICBzZWxmLmJ1Y2tldCA9IGJ1Y2tldAogICAgICAgICBzZWxmLl9jb3VudCA9IDAKKyAgICAg ICAgc2VsZi5fY2FjaGVkX3Rva2VuID0gTm9uZQorCisgICAgICAgIGlmIG5vdCBzZWxmLmNyZWRl bnRpYWxzOgorICAgICAgICAgICAgcmFpc2UgVmFsdWVFcnJvcignTm8gUzMgY3JlZGVudGlhbHMg cGFzc2VkJykKIAogICAgICAgICAjIE9ubHkgbW9kaWZ5IHRoZSBsaWZlY3ljbGUgb2YgdGhlIGJ1 Y2tldCBpZiB3ZSdyZSBlZGl0aW5nIHRoZSBzY2hlbWEKICAgICAgICAgaWYgb3MuZW52aXJvblsn Q1FMRU5HX0FMTE9XX1NDSEVNQV9NQU5BR0VNRU5UJ10gPT0gJzEnOgpAQCAtODgsNyArOTIsOCBA QCBjbGFzcyBTM0FyY2hpdmVyKEFyY2hpdmVyKToKIAogICAgIGRlZiBfX2VudGVyX18oc2VsZik6 CiAgICAgICAgIHNlbGYuX2NvdW50ICs9IDEKLSAgICAgICAgaWYgbm90IHNlbGYucmVzb3VyY2Vz OgorICAgICAgICBpZiBzZWxmLl9jYWNoZWRfdG9rZW4gIT0gc2VsZi5jcmVkZW50aWFscy5hd3Nf c2Vzc2lvbl90b2tlbjoKKyAgICAgICAgICAgIHNlbGYuX2NhY2hlZF90b2tlbiA9IHNlbGYuY3Jl ZGVudGlhbHMuYXdzX3Nlc3Npb25fdG9rZW4KICAgICAgICAgICAgIHNlbGYucmVzb3VyY2VzID0g Ym90bzMucmVzb3VyY2UoCiAgICAgICAgICAgICAgICAgc2VydmljZV9uYW1lPSdzMycsCiAgICAg ICAgICAgICAgICAgcmVnaW9uX25hbWU9c2VsZi5jcmVkZW50aWFscy5yZWdpb25fbmFtZSwKQEAg LTEwMSw2ICsxMDYsNyBAQCBjbGFzcyBTM0FyY2hpdmVyKEFyY2hpdmVyKToKICAgICAgICAgc2Vs Zi5fY291bnQgLT0gMQogICAgICAgICBpZiBzZWxmLl9jb3VudCA8PSAwOgogICAgICAgICAgICAg c2VsZi5yZXNvdXJjZXMgPSBOb25lCisgICAgICAgICAgICBzZWxmLl9jYWNoZWRfdG9rZW4gPSBO b25lCiAgICAgICAgICAgICBzZWxmLl9jb3VudCA9IDAKIAogICAgIGRlZiBzYXZlKHNlbGYsIGFy Y2hpdmUsIHJldGFpbl9mb3I9Tm9uZSk6Cg==