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

Change Azure PR deploy storage #16304

Closed
wants to merge 2 commits into from

Conversation

ecraig12345
Copy link
Member

@ecraig12345 ecraig12345 commented Dec 29, 2020

Switch PR deploys and perf results to use the fluentuipr Azure storage account (and an Azure connection which only has access to that storage account), and simplify the URLs. This means PR deploy URLs will now be like https://fluentuipr.z22.web.core.windows.net/pull/#####/ instead of http://fabricweb.z5.web.core.windows.net/pr-deploy-site/refs/pull/#####/merge/. (Or for CI builds it's https://fluentuipr.z22.web.core.windows.net/heads/branchname/.)

Also switch the release build to only upload artifacts, and use a separate internal release pipeline to upload those artifacts to fabricweb blob storage.

After this PR merges, I'll be changing permission settings so that pipelines (particularly in the public ADO project) don't have permission to upload to fabricweb.

Additional related changes:

  • Always use Build.SourceBranch (not Build.SourceVersion) in deploy URLs. This makes the structure easier to navigate in the storage explorer and easier for humans to parse.
    • Move northstar screener deployed build under the standard base path under pull/####/react-northstar-screener (or heads/branchname/react-northstar-screener)
  • Move demo image upload into the release job, since it still needs to upload to fabricweb storage

@DustyTheBot
Copy link

DustyTheBot commented Dec 29, 2020

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Generated by 🚫 dangerJS against cac9623

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 29, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7e1fd8b:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@size-auditor
Copy link

size-auditor bot commented Dec 29, 2020

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 7f3b1fa3fac2b558c9c8b63a4b55ac6da5a4687d (build)

@layershifter
Copy link
Member

@ecraig12345 we need apply the "404 page" trick for new storage with a redirect otherwise N* docs will be broken on deep URLs:

https://fluentuipr.z22.web.core.windows.net/pr-deploy-site/refs/pull/16304/merge/react-northstar/maximize/card-example-focusable-grid/false

The requested content does not exist.


curl http://fabricweb.z5.web.core.windows.net/pr-deploy-site/refs/pull/16304/merge/react-northstar/maximize/card-example-focusable-grid/false

<!DOCTYPE html>
<html lang="en">
  <body>
    <script>
      var northstarMatch = location.href.match(/^(.*\/pr-deploy-site\/.*?\/react-northstar)\//);
      if (northstarMatch) {
        // For URLs within a react-northstar PR deploy site, store the actual URL
        sessionStorage.redirect = location.href;
        // and redirect to the root of the site (which will then look at sessionStorage.redirect)
        var meta = document.createElement('meta');
        meta.setAttribute('http-equiv', 'refresh');
        meta.setAttribute('content', "0;URL='" + northstarMatch[1] + "'");
        document.head.appendChild(meta);
      } else {
        document.body.innerHTML = '<h1>The requested content does not exist.</h1>';
      }
    </script>
  </body>
</html>

@layershifter
Copy link
Member

so I suppressed the message for now

I suggest to update the database anyway to avoid this warning.

@ling1726 ling1726 closed this Dec 29, 2020
@ling1726 ling1726 reopened this Dec 29, 2020
@ling1726
Copy link
Member

@ecraig12345 we need apply the "404 page" trick for new storage with a redirect otherwise N* docs will be broken on deep URLs:

https://fluentuipr.z22.web.core.windows.net/pr-deploy-site/refs/pull/16304/merge/react-northstar/maximize/card-example-focusable-grid/false

The requested content does not exist.

curl http://fabricweb.z5.web.core.windows.net/pr-deploy-site/refs/pull/16304/merge/react-northstar/maximize/card-example-focusable-grid/false

<!DOCTYPE html>
<html lang="en">
  <body>
    <script>
      var northstarMatch = location.href.match(/^(.*\/pr-deploy-site\/.*?\/react-northstar)\//);
      if (northstarMatch) {
        // For URLs within a react-northstar PR deploy site, store the actual URL
        sessionStorage.redirect = location.href;
        // and redirect to the root of the site (which will then look at sessionStorage.redirect)
        var meta = document.createElement('meta');
        meta.setAttribute('http-equiv', 'refresh');
        meta.setAttribute('content', "0;URL='" + northstarMatch[1] + "'");
        document.head.appendChild(meta);
      } else {
        document.body.innerHTML = '<h1>The requested content does not exist.</h1>';
      }
    </script>
  </body>
</html>

That is also in the existing 404 in the the fabricweb storage account

@ecraig12345
Copy link
Member Author

Thanks for the note on the 404 page, I knew there was something else we needed to do but for some reason when I downloaded the existing 404 page from fabricweb storage it didn't include the script block? That's fixed now.

@ecraig12345
Copy link
Member Author

so I suppressed the message for now

I suggest to update the database anyway to avoid this warning.

From the error message it wasn't clear to me whether this was a real issue and if so, whether the suggested fix was actually applicable to our situation. I was also hesitant to mess with this dependency since it's specific to northstar, which I'm less familiar with. So I'd prefer if one of you could fix it @layershifter or @ling1726.

@fabricteam
Copy link
Collaborator

fabricteam commented Dec 30, 2020

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 816 817 5000
BaseButtonCompat mount 886 882 5000
Breadcrumb mount 42877 42777 5000
Checkbox mount 1530 1512 5000
CheckboxBase mount 1238 1218 5000
ChoiceGroup mount 4685 4718 5000
ComboBox mount 954 948 1000
CommandBar mount 10028 10102 1000
ContextualMenu mount 6009 6053 1000
DefaultButtonCompat mount 1079 1139 5000
DetailsRow mount 3626 3624 5000
DetailsRowFast mount 3615 3556 5000
DetailsRowNoStyles mount 3358 3451 5000
Dialog mount 1463 1448 1000
DocumentCardTitle mount 1773 1792 1000
Dropdown mount 3244 3258 5000
FocusTrapZone mount 1781 1784 5000
FocusZone mount 1830 1824 5000
IconButtonCompat mount 1685 1724 5000
Label mount 327 345 5000
Layer mount 1722 1735 5000
Link mount 460 474 5000
MakeStyles mount 1913 1925 50000
MenuButtonCompat mount 1487 1449 5000
MessageBar mount 1992 2012 5000
Nav mount 3225 3156 1000
OverflowSet mount 1037 1012 5000
Panel mount 1413 1432 1000
Persona mount 837 854 1000
Pivot mount 1386 1398 1000
PrimaryButtonCompat mount 1275 1261 5000
Rating mount 7367 7445 5000
SearchBox mount 1285 1256 5000
Shimmer mount 2493 2522 5000
Slider mount 1861 1878 5000
SpinButton mount 5154 4939 5000
Spinner mount 414 399 5000
SplitButtonCompat mount 3074 3065 5000
Stack mount 483 482 5000
StackWithIntrinsicChildren mount 1530 1497 5000
StackWithTextChildren mount 4390 4419 5000
SwatchColorPicker mount 10105 10238 5000
Tabs mount 1389 1399 1000
TagPicker mount 2850 2767 5000
TeachingBubble mount 11468 11416 5000
Text mount 403 419 5000
TextField mount 1343 1362 5000
ThemeProvider mount 2158 2140 5000
ThemeProvider virtual-rerender 666 651 5000
Toggle mount 816 783 5000
button mount 635 643 5000
buttonNative mount 113 118 5000

Perf Analysis (Fluent)

⚠️ 2 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
AttachmentMinimalPerf.default 173 160 1.08:1 analysis
ButtonMinimalPerf.default 176 172 1.02:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.43 0.51 0.84:1 2000 859
🦄 Button.Fluent 0.12 0.2 0.6:1 5000 592
🔧 Checkbox.Fluent 0.65 0.35 1.86:1 1000 654
🎯 Dialog.Fluent 0.16 0.21 0.76:1 5000 817
🔧 Dropdown.Fluent 3.01 0.4 7.53:1 1000 3009
🔧 Icon.Fluent 0.15 0.06 2.5:1 5000 748
🦄 Image.Fluent 0.07 0.13 0.54:1 5000 372
🔧 Slider.Fluent 1.58 0.45 3.51:1 1000 1583
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 366
🦄 Tooltip.Fluent 0.11 0.88 0.13:1 5000 572

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PortalMinimalPerf.default 183 166 1.1:1
ChatWithPopoverPerf.default 489 453 1.08:1
RefMinimalPerf.default 260 241 1.08:1
ListNestedPerf.default 609 576 1.06:1
ReactionMinimalPerf.default 426 403 1.06:1
CardMinimalPerf.default 583 553 1.05:1
LabelMinimalPerf.default 445 424 1.05:1
Icon.Fluent 748 713 1.05:1
HeaderMinimalPerf.default 384 371 1.04:1
LoaderMinimalPerf.default 752 724 1.04:1
PopupMinimalPerf.default 722 696 1.04:1
TreeMinimalPerf.default 798 769 1.04:1
Checkbox.Fluent 654 628 1.04:1
Dialog.Fluent 817 789 1.04:1
AccordionMinimalPerf.default 160 155 1.03:1
DialogMinimalPerf.default 804 782 1.03:1
DropdownManyItemsPerf.default 755 733 1.03:1
ItemLayoutMinimalPerf.default 1289 1254 1.03:1
ListWith60ListItems.default 953 923 1.03:1
MenuButtonMinimalPerf.default 1589 1546 1.03:1
IconMinimalPerf.default 684 667 1.03:1
TextAreaMinimalPerf.default 488 472 1.03:1
ButtonUseCssPerf.default 830 810 1.02:1
CarouselMinimalPerf.default 459 448 1.02:1
CheckboxMinimalPerf.default 2929 2873 1.02:1
ListMinimalPerf.default 509 501 1.02:1
SkeletonMinimalPerf.default 390 384 1.02:1
SplitButtonMinimalPerf.default 3774 3690 1.02:1
TextMinimalPerf.default 367 359 1.02:1
Slider.Fluent 1583 1558 1.02:1
AttachmentSlotsPerf.default 1130 1118 1.01:1
DatepickerMinimalPerf.default 47394 46982 1.01:1
GridMinimalPerf.default 357 353 1.01:1
LayoutMinimalPerf.default 424 419 1.01:1
MenuMinimalPerf.default 862 852 1.01:1
ProviderMinimalPerf.default 1035 1028 1.01:1
RadioGroupMinimalPerf.default 453 450 1.01:1
StatusMinimalPerf.default 744 740 1.01:1
CustomToolbarPrototype.default 3944 3890 1.01:1
ToolbarMinimalPerf.default 957 948 1.01:1
Button.Fluent 592 587 1.01:1
Tooltip.Fluent 572 567 1.01:1
AnimationMinimalPerf.default 416 414 1:1
BoxMinimalPerf.default 366 367 1:1
ButtonOverridesMissPerf.default 1699 1696 1:1
ButtonSlotsPerf.default 601 602 1:1
FormMinimalPerf.default 433 433 1:1
ImageMinimalPerf.default 407 405 1:1
TableMinimalPerf.default 429 431 1:1
TooltipMinimalPerf.default 824 824 1:1
VideoMinimalPerf.default 637 638 1:1
Avatar.Fluent 859 862 1:1
Dropdown.Fluent 3009 3007 1:1
Text.Fluent 366 365 1:1
ButtonUseCssNestingPerf.default 1103 1118 0.99:1
DividerMinimalPerf.default 376 378 0.99:1
DropdownMinimalPerf.default 2981 2998 0.99:1
EmbedMinimalPerf.default 4117 4157 0.99:1
ProviderMergeThemesPerf.default 2092 2117 0.99:1
SegmentMinimalPerf.default 361 366 0.99:1
SliderMinimalPerf.default 1567 1580 0.99:1
TableManyItemsPerf.default 2119 2137 0.99:1
ChatDuplicateMessagesPerf.default 415 425 0.98:1
ChatMinimalPerf.default 630 641 0.98:1
FlexMinimalPerf.default 306 313 0.98:1
HeaderSlotsPerf.default 780 800 0.98:1
InputMinimalPerf.default 1304 1326 0.98:1
AvatarMinimalPerf.default 476 491 0.97:1
Image.Fluent 372 382 0.97:1
AlertMinimalPerf.default 295 306 0.96:1
ListCommonPerf.default 637 663 0.96:1
TreeWith60ListItems.default 202 216 0.94:1

@ecraig12345 ecraig12345 reopened this Dec 31, 2020
Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for addressing comments 👍

@layershifter
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@layershifter
Copy link
Member

image

CI still fails with unclear timeout. @ecraig12345 any ideas?

@ecraig12345
Copy link
Member Author

On second thought, closing this and making a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants