[iOS] Update README.md

Issue

none

Overview (Required)

Update README.md

The first time I cloned it, the build failed.
It's listed in the Workaround section, but I think it needed to be in the GetStarted section because it's important information.
If you don't mind, I would appreciate it if you could merge it in.
Otherwise, you can close it. 🙇♂
Thanks!
Links

Preview README.md

Screenshot

Before
After

That's Good! Fixed! https://github.com/DroidKaigi/conference-app-2022/pull/453/commits/22ffd7b9e6d8989e76ce6c647acf501fba729b29
Thanks for your suggestions!


Improve using of graph transform tool

Please make sure that this is a documentation issue. As per our GitHub Policy, we only address code/doc bugs, performance issues, feature requests and build/installation issues on GitHub. tag:doc_template
System information

TensorFlow version: 1.12.0
Doc Link: https://github.com/tensorflow/tensorflow/tree/master/tensorflow/tools/graph_transforms

Describe the documentation issue
As newbie to Tensorflow it quite confusing seeing that you need to bazel build something, all I had to do before this point is pip install. To be more specific, what is bazel? Why do I need it to use GTT? Why is GTT not available from pip installation? Why when I'm going to Build from source page it encourages me that you providing pre-built packages, where is GTT then?

Hi @andrhua, we tend to make binaries available for things with stable api's, that we want to encourage the use of. There's no binary for this and it's never mentioned on tensorflow.org, which likely puts it in the category of internal details that regular users shouldn't rely-on.
Hey @suharshs, you're more familiar with the graph-transformation landscape. Do you know what the future holds for this tool?

Nagging Assignees @MarkDaoust, @suharshs: It has been 14 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.


[FeatureRequest] Let us choose Dark/Light themes manually

I am assuming "Dynamic Color" setting changes Dark/Light mode but in my case there is no Dark theme of the phone (Huawei P9 Lite - Android 8) so it is always Light theme.
Letting us choose Dark/Light theme options can fix this issue.

Hi,is this issue still available?Can I take this?Please assign to me.

@LLinlll :clap:

Hi,is this issue still available?Can I take this?Please assign to me.

Certainly! Thanks for your interest!
Feel free to open a PR once you are done!

@deltazefiro I would like to ask if I can add a button to change the theme directly on the main page, or would you prefer it to be on the setting page? Although I'd prefer to be on the main page, the setting page seems too crowded.

@deltazefiro I would like to ask if I can add a button to change the theme directly on the main page, or would you prefer it to be on the setting page? Although I'd prefer to be on the main page, the setting page seems too crowded.

I think it would make sense to put it just below "Dynamic Color" setting.

@deltazefiro I would like to ask if I can add a button to change the theme directly on the main page, or would you prefer it to be on the setting page? Although I'd prefer to be on the main page, the setting page seems too crowded.

I think it would make sense to put it just below "Dynamic Color" setting.

Sorry，I did not find where is the"Dynamic Color" setting.

Sorry，I did not find where is the"Dynamic Color" setting.

It's part of "Misc" settings.

I would like to ask if I can add a button to change the theme directly on the main page, or would you prefer it to be on the setting page?

I'd also suggest placing it beneath the "Dynamic Color" setting - in the setting page.
Android 10 introduced a global dark theme switch. Since over 70% of Android users have Android 10 or newer, switching themes isn't frequent enough to warrant a spot on the main page.

the setting page seems too crowded.

Absolutely! I'm planning a refactor to divide the settings across multiple pages and use data binding for enhanced maintainability. Perhaps in v0.10.

One more suggestion add a Pure Black Theme for Amoled Device User which can save some battery. Thanks

@LLinlll  Hey , Are you working on this PR?


How to make PriceCoordinate line dashed?

Hey, I need to have a line on a chart indicating a price coordinate with a dashed style.
What I want:

What I have:

Looking at https://github.com/rrag/react-stockcharts/blob/93b90cb46ed62136c6a0975777cbbdcc11e8ca28/src/lib/coordinates/PriceCoordinate.js I can't seem to find a prop for it. Ideally, I'd like PriceCoordinate to have support for the strokeDasharray like CrossHairCursor has.
Is there any way to style the line right now until the prop support arrives? Thank you!

0.7.1 now published includes support for this http://rrag.github.io/react-stockcharts/documentation.html#/price_marker_demonstration


Update Pages “blog”

Automatically generated by Netlify CMS

👷 Deploy Preview for serene-melomakarona-ce295f processing.

Name
Link

🔨 Latest commit
6553f02ad596f7cb3c7995cac1d45acc493d996e

🔍 Latest deploy log
https://app.netlify.com/sites/serene-melomakarona-ce295f/deploys/649b55f45fad310008f36339


Installer failed with exit code: 2008

Brief description of your issue
I am checking the options and how to use the Winget commands to upgrade applications and when trying to use the upgrade command for Windows ADK, that is the one Winget is showing me available, I am getting the error. I searched for that error here but I am not seeing it, so not sure if this is a known issue or something new
Steps to reproduce
PS C:\Windows\system32> winget upgrade
Name                                  Id                   Version      Available    Source
Windows Assessment and Deployment Kit Microsoft.WindowsADK 10.1.14393.0 10.1.22000.1 winget
1 upgrades available.
PS C:\Windows\system32> winget upgrade --id Microsoft.WindowsADK
Found Windows Assessment and Deployment Kit [Microsoft.WindowsADK] Version 10.1.22000.1
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
Downloading https://download.microsoft.com/download/1/f/d/1fd2291e-c0e9-4ae0-beae-fbbe0fe41a5a/adk/adksetup.exe
██████████████████████████████  2.08 MB / 2.08 MB
Successfully verified installer hash
Starting package install...
Installer failed with exit code: 2008
Expected behavior
Get the application updated
Actual behavior
getting 2008 exit code error
Environment
PS C:\Windows\system32> winget --info
Windows Package Manager v1.1.12986
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.18363.1977 (1909)
Package: Microsoft.DesktopAppInstaller v1.16.12986.0

Steps to replicate:
PS M:\> winget install Microsoft.WindowsADK --version 10.1.19041.1
Found Windows Assessment and Deployment Kit - Windows 10 [Microsoft.WindowsADK] Version 10.1.19041.1
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
Downloading https://download.microsoft.com/download/8/6/c/86c218f3-4349-4aa5-beba-d05e48bbc286/adk/adksetup.exe
  ██████████████████████████████  1.84 MB / 1.84 MB
Successfully verified installer hash
Starting package install...
Successfully installed

PS M:\> winget upgrade Microsoft.WindowsADK
Found Windows Assessment and Deployment Kit [Microsoft.WindowsADK] Version 10.1.22000.1
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
Downloading https://download.microsoft.com/download/1/f/d/1fd2291e-c0e9-4ae0-beae-fbbe0fe41a5a/adk/adksetup.exe
  ██████████████████████████████  2.08 MB / 2.08 MB
Successfully verified installer hash
Starting package install...
Installer failed with exit code: 2008

Change InstallerType to burn
PS M:\> winget upgrade -m .\m\Microsoft\WindowsADK\10.1.22000.1\
Found Windows Assessment and Deployment Kit [Microsoft.WindowsADK] Version 10.1.22000.1
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
Downloading https://download.microsoft.com/download/1/f/d/1fd2291e-c0e9-4ae0-beae-fbbe0fe41a5a/adk/adksetup.exe
  ██████████████████████████████  2.08 MB / 2.08 MB
Successfully verified installer hash
Starting package install...
Installer failed with exit code: 2008
Installer log is available at: C:\Users\User\AppData\Local\Packages\Microsoft.DesktopAppInstaller_8wekyb3d8bbwe\LocalState\DiagOutputDir\WinGet-Microsoft.WindowsADK.10.1.22000.1-2021-12-23-22-51-56.467.log

WinGet-2021-12-23-22-51-55.280.log
WinGet-Microsoft.WindowsADK.10.1.22000.1-2021-12-23-22-51-56.467.log
Probably this line?
[10B0:0DEC][2021-12-23T22:51:57]i000: ERROR: MSI replacement servicing is not supported

Trying to open the exe

Add UpgradeBehavior: uninstallPrevious to the manifest
PS M:\> winget upgrade -m .\m\Microsoft\WindowsADK\10.1.22000.1\
Found Windows Assessment and Deployment Kit [Microsoft.WindowsADK] Version 10.1.22000.1
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
Downloading https://download.microsoft.com/download/1/f/d/1fd2291e-c0e9-4ae0-beae-fbbe0fe41a5a/adk/adksetup.exe
  ██████████████████████████████  2.08 MB / 2.08 MB
Successfully verified installer hash
Starting package uninstall...
Successfully uninstalled
Starting package install...
Installer failed with exit code:<PHONE_NUMBER>
Installer log is available at: C:\Users\User\AppData\Local\Packages\Microsoft.DesktopAppInstaller_8wekyb3d8bbwe\LocalState\DiagOutputDir\WinGet-Microsoft.WindowsADK.10.1.22000.1-2021-12-23-23-01-09.384.log

WinGet-2021-12-23-23-00-39.247.log
WinGet-Microsoft.WindowsADK.10.1.22000.1-2021-12-23-23-01-09.384.log
WinGet-Microsoft.WindowsADK.10.1.22000.1-2021-12-23-23-01-09.384_000_package_ToolkitDocumentation_x86_en_us.log
WinGet-Microsoft.WindowsADK.10.1.22000.1-2021-12-23-23-01-09.384_000_package_ToolkitDocumentation_x86_en_us_rollback.log
WinGet-Microsoft.WindowsADK.10.1.22000.1-2021-12-23-23-01-09.384_001_package_WindowsDeploymentToolsEnvironment_x86_en_us.log
WinGet-Microsoft.WindowsADK.10.1.22000.1-2021-12-23-23-01-09.384_001_package_WindowsDeploymentToolsEnvironment_x86_en_us_rollback.log

any updates?

I'm receiving this error as well when attempting to upgrade the WindowsSDK.
(2/2) Found Windows Software Development Kit [Microsoft.WindowsSDK] Version 10.0.22621.1
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
Successfully verified installer hash
Starting package install...
Installer failed with exit code: 2008

❯ winget --info
Windows Package Manager v1.3.2091
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.22000.856
System Architecture: X64
Package: Microsoft.DesktopAppInstaller v1.18.2091.0

Error in log file:
[2D3C:5990][2022-08-10T08:16:57]i000: Setting string variable 'VS2022INSTALL' to value 'C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\Common7\IDE'
[2D3C:5990][2022-08-10T08:16:57]i000: ERROR: MSI replacement servicing is not supported
[2D3C:5990][2022-08-10T08:16:57]i000: View is requesting shutdown
[2D3C:5990][2022-08-10T08:16:57]i199: Detect complete, result: 0x0
[2D3C:5990][2022-08-10T08:16:57]i500: Shutting down, exit code: 0x7d8
....
....
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: SystemFolder = C:\WINDOWS\SysWOW64\
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: UCRTEXISTS = 1
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: UCRTVERSION = 10.0.22000.1
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: VersionNT64 = <IP_ADDRESS>
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: VS2022INSTALL = C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\Common7\IDE
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleAction = 5
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleElevated = 1
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleInstalled = 1
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleLastUsedSource = C:\ProgramData\Microsoft\VisualStudio\Packages\Win11SDK_10.0.22621,version=10.0.22621.0,productarch=neutral\
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleLog = C:\Users\user\AppData\Local\Temp\windowssdk\Windows_Software_Development_Kit___Windows_10.0.22621.1_20220810081657.log
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleManufacturer = Microsoft Corporation
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleName = Windows Software Development Kit - Windows 10.0.22621.1
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleOriginalSource = C:\WINDOWS\Temp\{E8F15567-E078-42AF-A100-B3823D6B2D09}\.cr\winsdksetup.exe
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleOriginalSourceFolder = C:\ProgramData\Microsoft\VisualStudio\Packages\Win11SDK_10.0.22621,version=10.0.22621.0,productarch=neutral\
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleProviderKey = {fcb5e4cc-2739-4812-97fd-f3b7c5d151ae}
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleSourceProcessFolder = C:\Users\user\AppData\Local\Temp\WinGet\Microsoft.WindowsSDK.10.0.22621.1\
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleSourceProcessPath = C:\Users\user\AppData\Local\Temp\WinGet\Microsoft.WindowsSDK.10.0.22621.1\winsdksetup.exe
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleTag =
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleUILevel = 2
[2D3C:5990][2022-08-10T08:16:57]i410: Variable: WixBundleVersion = 10.1.22621.1

Unable to even uninstall the package via winget:
❯ winget uninstall --id Microsoft.WindowsSDK
No installed package found matching input criteria.

❯ winget upgrade
Name                                                    Id                   Version          Available    Source
-----------------------------------------------------------------------------------------------------------------
Windows Software Development Kit - Windows 10.0.20348.1 Microsoft.WindowsSDK < 10.0.22000.832 10.0.22621.1 winget

@denelon have you notified someone at Microsoft about this or do you have a client fix in mind for this?
For anyone encountering this issue leave a thumps up on the original post.

I created a PR with the changes OP suggested: #69265

Same problem for me...

I'm reaching out to see if I can find the team who owns WindowsADK to understand what "error 2008" means so we can potentially map to an expected return code, or look at what the behavior should be.

I had this problem today.
I made sure all my Windows SDKs were uninstalled from Settings->Apps and from within the VS installer.
And would get the following if I downloaded the SDK and ran the installer

Which was weird because Windows SDK 10.0.22621.755 didn't show up in VS, winget or Settings -> Apps

search the registry for 10.0.22621.755
find "C:\ProgramData\Package Cache{bbdcfd1a-000e-4b9d-af4f-07049d1e9a47}\winsdksetup.exe" /modify
run an admin prompt and go to that folder
run winsdksetup.exe /modify to remove
winget install Microsoft.WindowsSDK -s winget
Install is successfull
Launch VS and open my solution
Get an error in the VS UI that UWP stuff is missing - but it prompts to download
Download and install (close VS, let visual studio installer do the thing)

Launch VS and open my solution - it builds and runs

I had this problem today.
I made sure all my Windows SDKs were uninstalled from Settings->Apps and from within the VS installer.
And would get the following if I downloaded the SDK and ran the installer
Which was weird because Windows SDK 10.0.22621.755 didn't show up in VS, winget or Settings -> Apps

search the registry for 10.0.22621.755
find "C:\ProgramData\Package Cache{bbdcfd1a-000e-4b9d-af4f-07049d1e9a47}\winsdksetup.exe" /modify
run an admin prompt and go to that folder
run winsdksetup.exe /modify to remove
winget install Microsoft.WindowsSDK -s winget
Install is successfull
Launch VS and open my solution
Get an error in the VS UI that UWP stuff is missing - but it prompts to download
Download and install (close VS, let visual studio installer do the thing)

Launch VS and open my solution - it builds and runs

This issue seems to still be occurring in Winget ... & VS - they're not recognizing the newer release (well, actually, winget is not recognizing a lot of different packages that are actually up to date) ... but, at the same time, ...both will recognize the newer release...(?) Winget wants you to uninstall Microsoft.WindowsSDK 10.0.22621.755, yet Winget shows 10.0.22621.2 as the most up to date & available release, & whatever you do, it spits out the error '2008' ... Strangely enough, VS wants you to uninstall 10.0.22621.755 too...? Okay....
winget uninstall Microsoft.WindowsSDK --version 10.0.22621.755
It will uninstall successfully. Then click & download from the MS dev page link (OR! The ~direct installer link winget will also provide you) to install Microsoft.WindowsSDK once again, &, if you have a keen eye, then you will notice you are installing the same v10.0.22621.755 that was just uninstalled! ... The one benefit is that it won't appear in the list generated by winget upgrade --include-unknown from then on.

This ADKSetup and SDKsetup is a pretty big trouble because it can’t upgrade until you uninstall previous version. Exclude for WDKSetup

This issue has been resolved now that uninstallPrevious has been added to the package manifest: https://github.com/microsoft/winget-pkgs/pull/86259
I also verified on a clean machine that installing an older version and upgrading now succeeds and no longer returns a 2008 exit code.


Hide 'Current user' button next to the address bar

Is that possible to hide this button, that allows to sing in the Google account, for those who uses Chromium without singing it to it?

It doesn't look like there's any configuration flag for it. I think the only way is to modify the source code.

@sgtpep Is this what you're asking for?

It is added by patches/extra/ungoogled-chromium/add-flag-to-show-avatar-button.patch

@PF4Public Exactly what I needed. Thank you so much!

Huh.

Come to think of it, could we / should we probably change the name of this flag and reword its description, so that its purpose would be clearer and more obvious, for it to be easier to find? I do use this flag, which is obvious from a screenshot above, but if I didn't, I likely wouldn't be searching for an "avatar" or "button", I would have been searching for an "account", "user", "profile" etc.

The upstream calls it "avatar toolbar button".

And we may call it whatever we want:
https://github.com/Eloston/ungoogled-chromium/blob/4f574006aca69964995acfa0718e7097ce36c059/patches/extra/ungoogled-chromium/add-flag-to-show-avatar-button.patch#L27-L30

I'd be okay with adding to the title and description. The flag name itself should remain since it's more of a technical detail.

The flag name itself should remain since it's more of a technical detail.

Good point, some people might be already using it and changing the flag name will break this functionality for them.
I'll do a PR soon.


pal: Add support for displayable fp16 swapchain format.

Maps VK_FORMAT_R16_G16_B16_A16_SFLOAT to amdgpu-kms format
DRM_FORMAT_XBGR16161616F. Linux 5.8 amdgpu-kms supports
this format for scanout and display on gpu's with DCE-11.2
display engine (Polaris family) and later generations.
Linux kernel 5.8 mainline commit for this new feature is
2a5195dca0b7058e65443416a20680be985d8753
("drm/amd/display: Expose support for xBGR ordered fp16 formats.")
Successfully tested on Polaris DCE-11.2 and Raven Ridge DCN-1.
Signed-off-by: Mario Kleiner<EMAIL_ADDRESS>

Can one of the admins verify this patch?

ok to test

test this please

retest this please

Test summary for commit 980289e4a1d5eac320fe19fd664a5900e45c268e
Driver commits used in build
CWPACK: amd-master 7387247eb9889ddcabbc1053b9c2052e253b088e
METROHASH: amd-master 712f76fee75d69b23a1ea8f6465752c3ccaaf9a2
LLPC: dev d0d1016eee8ef5d32f73fa5da58c30c8a3e1aea7
SPVGEN: dev 8dc855026f2502ab3f45dadaf0bb802a57d6ad60
XGL: dev f4a992dd7e556ed5e7e2ffa2c830f1cd79bd4596
LLVM-PROJECT: amd-gfx-gpuopen-dev c2ea9a11546d054e3918c44ac3b3ad930686cd79

CTS tests (Failed: 0/130900)Built with branch Vulkan-CTS-<IP_ADDRESS>-fetch-sources-fix
Ubuntu 20.04, Gfx10
Passed: 22784/43839 (52.0%)
Failed: 0/43839 (0.0%)
Skipped: 21055/43839 (48.0%)

Ubuntu 18.04, Gfx9
Passed: 22783/43843 (52.0%)
Failed: 0/43843 (0.0%)
Skipped: 21060/43843 (48.0%)

Rhel 8.2, Gfx8
Passed: 21609/43218 (50.0%)
Failed: 0/43218 (0.0%)
Skipped: 21609/43218 (50.0%)

I  changed the order of commits in this force-pushed update, as the  'max bpc' patch seems to be fine as is. Other than the code style fix, the patches are the same. Thanks.

retest this please

Test summary for commit 66a81660022bbbb207ec63c5ab6f0c5b5703f121
Driver commits used in build
CWPACK: amd-master 7387247eb9889ddcabbc1053b9c2052e253b088e
METROHASH: amd-master 712f76fee75d69b23a1ea8f6465752c3ccaaf9a2
LLPC: dev 26120c68e0195722784e496b05e6ab57e21642c1
SPVGEN: dev 8dc855026f2502ab3f45dadaf0bb802a57d6ad60
XGL: dev f4a992dd7e556ed5e7e2ffa2c830f1cd79bd4596
LLVM-PROJECT: amd-gfx-gpuopen-dev ea979e90c306a6ddec5db687e9c8d33b5289a23c

CTS tests (Failed: 0/131499)Built with branch Vulkan-CTS-<IP_ADDRESS>-fetch-sources-fix
Ubuntu 20.04, Gfx10
Passed: 22784/43839 (52.0%)
Failed: 0/43839 (0.0%)
Skipped: 21055/43839 (48.0%)

Ubuntu 18.04, Gfx9
Passed: 22783/43843 (52.0%)
Failed: 0/43843 (0.0%)
Skipped: 21060/43843 (48.0%)

Rhel 8.2, Gfx8
Passed: 21609/43817 (49.3%)
Failed: 0/43817 (0.0%)
Skipped: 22208/43817 (50.7%)

Style fixes and adding an assert done in updated pull request. Thanks for the review.

@RobMartin-AMD ping.

retest this please

@RobMartin-AMD @JacobHeAMD ping? Would be nice to get this squared away, given that nobody voiced any objections against the general approach and implementation, only coding style nits. Thanks.

@JaxLinAMD , please help to merge this pull request.

retest this please

Test summary for commit e68b4ef46b8de7cfda99d0c37ff0f2273852e86b
Driver commits used in build
CWPACK: amd-master 7387247eb9889ddcabbc1053b9c2052e253b088e
METROHASH: amd-master 712f76fee75d69b23a1ea8f6465752c3ccaaf9a2
LLPC: dev f23bb234ee3ad79aad078995012edf93f8938676
SPVGEN: dev 8dc855026f2502ab3f45dadaf0bb802a57d6ad60
XGL: dev f4a992dd7e556ed5e7e2ffa2c830f1cd79bd4596
LLVM-PROJECT: amd-gfx-gpuopen-dev ccbc834ac4f92e63a9a003bd5a18cb9fcf3c6121

CTS tests (Failed: 0/131507)Built with branch Vulkan-CTS-<IP_ADDRESS>-fetch-sources-fix
Ubuntu 20.04, Gfx10
Passed: 22784/43840 (52.0%)
Failed: 0/43840 (0.0%)
Skipped: 21056/43840 (48.0%)

Ubuntu 18.04, Gfx9
Passed: 22783/43844 (52.0%)
Failed: 0/43844 (0.0%)
Skipped: 21061/43844 (48.0%)

Rhel 8.2, Gfx8
Passed: 21609/43823 (49.3%)
Failed: 0/43823 (0.0%)
Skipped: 22214/43823 (50.7%)

@kleinerm , could you add ModeSet back? this should not be removed

@kleinerm also, we prefer rebase than merge. So could you rebase your personal branch on top of dev branch, recover "ModeSet" I deleted and force-push to your branch again? you can also squash those commits into one single commit, that would make git history easy to understand. thanks!

I can try to do that, after some sleep. But do you mean squashing all of my commits into one? Even though the one that fixed 'max bpc' and the other one for fp16 are logically separate things - related, but independent?

Uhm, if they are independent two commits, then no need to squash:)

Ok. I guess i will keep the two patches separate, but add the ModeSet call back to the fp16 patch, so pulling this will also fix that bug from issue #46 i guess that's fine as you are assigned to that issue anyway, so we won't get new merge conflicts if i add it back here.

@kleinerm  FYI, I just submmited https://github.com/GPUOpen-Drivers/pal/commit/bebb2b0d0d70625d825cd386a1360154709487f2
add back "ModeSet(pImage)" you can go on with rebase

@kleinerm  Oops, please rebase and force-push again

@JaxLinAMD Ok, rebased, force-pushed, retested. Ready to pull. I go to bed now.

Test summary for commit 4b36745ccea3cd44cef9c47192751dc5b0b266e9
Driver commits used in build
CWPACK: amd-master 7387247eb9889ddcabbc1053b9c2052e253b088e
METROHASH: amd-master 712f76fee75d69b23a1ea8f6465752c3ccaaf9a2
LLPC: dev f18670dec0c800dd9a7ed514ced8843c4694217a
SPVGEN: dev 4865b574184e261d87e77277282e90e3be14c53a
XGL: dev 598c6832a4983f5b75b38a589fca5be80a2f3bb0
LLVM-PROJECT: amd-gfx-gpuopen-dev ccbc834ac4f92e63a9a003bd5a18cb9fcf3c6121

CTS tests (Failed: 0/131444)Built with branch Vulkan-CTS-<IP_ADDRESS>-fetch-sources-fix
Ubuntu 20.04, Gfx10
Passed: 22766/43819 (52.0%)
Failed: 0/43819 (0.0%)
Skipped: 21053/43819 (48.0%)

Ubuntu 18.04, Gfx9
Passed: 22765/43823 (51.9%)
Failed: 0/43823 (0.0%)
Skipped: 21058/43823 (48.1%)

Rhel 8.2, Gfx8
Passed: 21591/43802 (49.3%)
Failed: 0/43802 (0.0%)
Skipped: 22211/43802 (50.7%)

Test summary for commit a4690886ce42835c3da2c0fe8af9690579776aff
Driver commits used in build
CWPACK: amd-master 7387247eb9889ddcabbc1053b9c2052e253b088e
METROHASH: amd-master 712f76fee75d69b23a1ea8f6465752c3ccaaf9a2
LLPC: dev f18670dec0c800dd9a7ed514ced8843c4694217a
SPVGEN: dev 4865b574184e261d87e77277282e90e3be14c53a
XGL: dev 598c6832a4983f5b75b38a589fca5be80a2f3bb0
LLVM-PROJECT: amd-gfx-gpuopen-dev ccbc834ac4f92e63a9a003bd5a18cb9fcf3c6121

CTS tests (Failed: 0/131444)Built with branch Vulkan-CTS-<IP_ADDRESS>-fetch-sources-fix
Ubuntu 20.04, Gfx10
Passed: 22766/43819 (52.0%)
Failed: 0/43819 (0.0%)
Skipped: 21053/43819 (48.0%)

Ubuntu 18.04, Gfx9
Passed: 22765/43823 (51.9%)
Failed: 0/43823 (0.0%)
Skipped: 21058/43823 (48.1%)

Rhel 8.2, Gfx8
Passed: 21591/43802 (49.3%)
Failed: 0/43802 (0.0%)
Skipped: 22211/43802 (50.7%)

@kleinerm , Have a good night! Shouldn't it be " ret = ModeSet(pImage) != Result::Success;"? I guess you may want to change it again, currently it is" ModeSet(pImage)"
'

@JaxLinAMD No, ModeSet(pImage) is fine for now, doesn't really matter for this series, was just some small refinement. I don't want to go through another modify/compile/test cycle and delay this pull any longer.

@JaxLinAMD @JacobHeAMD Ok, got some sleep, changed my mind, added "ret = ModeSet(pImage) != Result::Success;" back into the commit. It doesn't really matter for the series, but a little improvement in error handling never hurts.
Retested, rebased, ready to pull. Please please can we get this in now before some random other commits from your side break it again and produce new merge conflicts?
Thanks.

retest this please

Thanks!


Update criterion

Noticed it was outdated.
Also passes tests when line 14 is removed but I left it because I have no idea why gen_hash_test.rs is a binary.

gen_hash_test.rs was to test asymptotic hash time issues in #132; it's easier to load it up in a profiler rather than as part of a larger test binary. It doesn't technically need to exist anymore if you'd like to delete it.

The build failure is due to error message changes on the latest nightly or perhaps a breaking change in trybuild. I can investigate this later if you don't feel like looking into it.

That lint output was changed in https://github.com/rust-lang/rust/pull/64290.
Added empty main function to avoid this lint.

Thanks!


Add support to immutable collections

It would be very useful that Castle Windsor could support System.Collections.Immutable in typed factories since it seems like returning an immutable list of implementations is more meaningful than returning mutable collections or a typed array:
    public interface ISampleFactory
    {
         ImmutableList<ISome> ListAllSomes();
    }

Hey,
The way I understand it is that the types that get returned from a factory need to be interfaces. This includes collections.
https://github.com/castleproject/Windsor/blob/master/docs/typed-factory-facility-interface-based.md
However even adding an IImmutableList instead of ImmutableList won't work because the constructor of ImmutableList is not public. On top of that, for this to work, CW would need to take a dependency on that particular package which not everyone needs.
For a quick/dirty solution, you could change it to IList and then create the Immutable list from that.
If you have enough time than you could create your own facility/resolver that could do this as well.

The OP was answered by @dalefrancis88


This allows importing Civ3 scenarios with non-default terrain names (…

…including defaults in non-English versions), and having them work.
There's a lookup table for the Civ3 types, and the Name property will henceforth only be used for display purposes.  I'm half-tempted to change its name to displayName to make that more clear, as otherwise I suspect we'll inadvertently introduce a regression at some point in the future.

Note: I need to re-gen the JSON file from my desktop; for some reason my laptop can't find the .NET 4.7.2 Targeting Pack to do so even after re-installing it.  But other than that, I think this is ready to review.

Huh, I didn't know it was possible to mark text files as binary in Git.  I agree that 13000 line diffs are unhelpful.  But the teams I'd worked on had resolved that by not committing generated files; we never knew about .gitattributes.  I've attempted to mark it as binary here, although it still shows up for now... not sure if that means I did it wrong, or it's because I marked it after making the JSON changes.  Going to merge anyway, before this branch picks up more conflicts.


Add Google Authentication

Description
Currently, the Alien Invasion Defense project uses manual authentication only. We need to implement Google authentication to enhance the user login experience.
Screenshots
No response
Any additional information?
Please assign this issue to me under gssoc-ext and hacktober fest labels.
What browser are you seeing the problem on?
Firefox, Chrome, Safari, Microsoft Edge
Checklist

[X] I have checked the existing issues
[X] I have read the Contributing Guidelines
[X] I'm a GSSoC'24-Extd contributor
[X] I'm a Hacktoberfest'24 contributor
[X] I am willing to work on this issue (optional)

I would like to work on this issue. Could you please assign me this task with difficulty label


Reimplement static-timing pass.

Supersedes #908. Closes #654.

Alright, here we go. Please see original discussion from #905.

All right, tests are running. Thanks for bearing with me on the GH actions stuff. Review coming up!

The test errors seem to come from Verilator complaining about constant comparison which is annoying. I remember having to deal with the old static timing pass as well. Let me know if you need help figuring out what assignments to generate.

Yep I saw that last night and just disabled it to test. I will take a look. I think it's because we generate guards like fsm_out >= 2'd0 & fsm_out < 2'd1 for enables in state 0, which causes verilator to complain the >= is always true. Is there a perferred pattern to avoid that? I was following the idea you laid out in the initial implementation and the issue.
The other tests are failing because I haven't gone through and added -d top-down-st to the "correctness dynamic" tests, like the issue said. I asked this in the other PR: what's the difference between "correctness dynamic" and "correctness static" tests? They target the same thing (at least in the current revision of this PR).

we should disable static-timing for those tests

Which tests are these exactly? I see the same regex for both "correctness dynamic": https://github.com/cucapra/calyx/blob/93ecebc8fbd782596cbde9b53d03a028a424d13e/runt.toml#L64-L67
And for the "correctness static timing": https://github.com/cucapra/calyx/blob/93ecebc8fbd782596cbde9b53d03a028a424d13e/runt.toml#L105-L106
(at least what is checked in right now)

I've pushed a few minor code fixes but the one big thing that the pass doesn't seem to currently handle is compiling the following kinds of program:
import "primitives/core.futil";

component main() -> () {
  cells {
    r0 = std_reg(32);
    r1 = std_reg(32);
    add0 = std_add(32);
    add1 = std_add(32);
    lt = std_lt(32);
  }
  wires {
    group incr_r0<"static"=1> {
      r0.write_en = 1'd1;
      r0.in = 32'd10;
      incr_r0[done] = r0.done;
    }
    group incr_r1<"static"=1> {
      r1.write_en = 1'd1;
      r1.in = 32'd10;
      incr_r1[done] = r1.done;
    }
  }
  control {
    seq {
      @static(2) seq { incr_r0; incr_r1; }
      seq { incr_r0; incr_r1; }
    }
  }
}

When running with:
% cargo run -- static-mixed.futil -p well-formed -p top-down-st

I would expect the pass to compile the first seq block but leave the second one for tdcc to compile. In general, the pass should opportunistically compile as much of the control sub-programs as possible.
One way to do this would be to modify the <seq|par|if>_calculate_states methods to recur when they cannot be fully compiled themselves. Some care needs to be taken to not traverse the IR twice.
Alternatively, you can require that only control programs where the top node has a @static annotation will be compiled and then override the start_if, start_seq, etc. methods.

Which tests are these exactly? I see the same regex for both "correctness dynamic":

That's because runt will use the same files to run both tests and expect their results to be the same. However, it will run each "suite" with different commands.

However, it will run each "suite" with different commands.

I think I understand the intention, but I'm pretty sure at the moment (in this PR) both "suites" run the same tests with the same commands, and I'm confused what should change. Is the idea to remove the top-down-st pass from the dynamic suite, so both suites test the same programs with different passes?

Ah, the -d pass tells the compiler to specifically disable a pass. So adding -d top-down-st to the dynamic test suite should do the trick.

Thanks that makes sense.
Regarding this comment:

I would expect the pass to compile the first seq block but leave the second one for tdcc to compile.

This makes sense, and I didn't catch it because in general, the tests I've done have had infer-static-timing before top-down-st, which would infer the entire control program to be static for this program. I will write a new test where the whole program is actually not static.
Regardless, it should be simple enough to do what you mentioned and attempt to compile as much of the program as possible. Right now, it is all-or-nothing.

Yeah, I think we'd want to pass -d infer-static-timing for the tests that print out the FSMs for static timing to ensure that it doesn't interfere with other things.

I looked into this, and if we want to disable infer-static-timing, we will also need to massage the example programs into the form it looks like after remove-comb-groups. If we just disable infer-static-timing, when remove-comb-groups runs, the enables for the generated group don't have the @static annotation. Is the idea for these to be unit tests with the IR in a state we would expect right before top-down-st?

I think my solution was to special case the guard generator to instead say: fsm_out == 2'd0 instead in those cases.

For this one, I ended up just dropping the comparison between fsm_out and 0. So if start state is 0 and end state is 2, instead of fsm_out >= 0 & fsm_out < 2, we'd say fsm_out < 2. We couldn't say fsm_out == 0 & fsm_out < 2, because fsm_out will only equal zero for the first of potentially several cycles we want the enable in question to be high. I think this works, since the correctness tests seem to pass.

adding -d top-down-st to the dynamic test suite

Sorry if I'm being dense, but -d appears to be an option to futil, not fud. Should I change these test suites to use a futil command?

I would expect the pass to compile the first seq block but leave the second one for tdcc to compile. In general, the pass should opportunistically compile as much of the control sub-programs as possible.

I've been thinking about this a bit, and I think it's a little more complicated than I initially thought. I can imagine compiling static sub-trees of the schedule, but I'm less sure how to stitch together potentially multiple static FSMs. Right now the pass replaces the top-level control program with a single group to implement the entire FSM. I guess we would need to replace each static sub-tree with an FSM, and in the case the entire control program wasn't static, those sub-FSMs would remain embedded in the larger control program? If that's the right angle to take here, I need to do some more thinking about how this fits with the current structure of the traversal and what I might need to change... I am only interested in programs that are fully-static for my use case, so would it be an option to land this pass in an "all-or-nothing" form, and re-work it to support partially static programs as a follow up? Or is this a requirement from the start?

if we just disable infer-static-timing, when remove-comb-groups runs, the enables for the generated group don't have the @static annotation.

Ah, got it. We can change remove-comb-groups to add the @static annotation on the generated groups. I'll handle this.

Is the idea for these to be unit tests with the IR in a state we would expect right before top-down-st?

Yes! We want to basically make sure that top-down-st compiles things correctly and triggers for all the cases we expect. Because it is an "optimization" (i.e. it triggers opportunistically), the unit tests should be clear about exactly which part of a control sub-program should be compiled.

Sorry if I'm being dense, but -d appears to be an option to futil, not fud. Should I change these test suites to use a futil command?

Ah, sorry, I forgot that these are run through fud. You can pass -s futil.flags ' -d infer-static-timing' and fud will invoke the compiler correctly. When in doubt, you can also run fud ... -n which runs fud in dry run mode and prints out the commands its going to run. Finally, runt -i 'correctness dynamic' -n will print out the commands runt will run for testing so you can copy paste them as needed for debugging.

I think this works, since the correctness tests seem to pass.

Seems pretty reasonable to me!

I can imagine compiling static sub-trees of the schedule, but I'm less sure how to stitch together potentially multiple static FSMs

So the high-level idea is that when you compile a control sub-program, you generate a group that represents all of the assignments needed to make the FSM work. Then, you can replace the whole control sub-program with just that group and the new tdst group you created will handle running everything correctly.
If this helps visualize it–the pass basically goes through the control program and opportunistically eliminates sub-programs and turns them into groups. tdcc handles compiling the remaining control program. The key idea is that because tdst is just generating groups, it doesn't matter how things are compiled within that group as long as it implements the right transitions. tdcc will just see another control program and compile it as needed.

I am only interested in programs that are fully-static for my use case, so would it be an option to land this pass in an "all-or-nothing" form, and re-work it to support partially static programs as a follow up? Or is this a requirement from the start?

I'm going to let you make the call on this. static-timing always worked in this opportunistic fashion so it'd be nice, but I'm also down for merging the MVP and incrementally improving it if it lets to make progress on the CIRCT side faster. Just let me know which one you prefer and we can merge accordingly.

Thanks for the pointers, I will get the tests fixed up.

if it lets to make progress on the CIRCT side faster

Nothing is blocked on this, so we might as well do this the right way from the start. I will try to take a crack at it today.

Woo! Fantastic, happy to jump on a call later if that will help.

Looked at some of the remaining test failures, and they're not the worst thing to see :)

Should we add the -d top-down-st there as well, to keep the old behavior?

Added that in a few places, and the test suite passes now.

Sweet! I'll play with this and see if there is a way to address the question about the seq logic you had in the original PR.

I am taking a look, and I think I'm seeing how to opportunistically handle the static parts. I'm not sure about "replace the whole control sub-program with just that group". Say I'm a seq node, and I want to replace one of the child sub-programs with an enable. What is the proper way to do this? I've only used the Visitor and Action::Change, so I guess this is what you meant earlier about overriding the start_seq, etc. methods?

So, this is where things get a little tricky. You need to know that all children of a node have a static annotation in order to statically compile it. The challenge is doing this without entirely traversing the set of children; in case one of them doesn't have a static annotation, you'll have to recur again.
The start_seq and finish_seq style methods allow you to traverse the control tree in the visitor and if you want to skip traversal, you can return Action::Skip or Action::SkipChildren. My first guess would be using start_* methods and when you can compile the program, using Action::Change(group) to replace the control tree. This is also where assuming infer-static-timing was run is useful because we can say that only control nodes that have @static will get compiled and we will not compute @static for you.
With this assumption, this would compile:
@static(2) seq { @static(1) one; @static(1) two }

but not
seq { @static(1) one; @static(1) two }

The other option is getting comp.control.borrow_mut() in the start method and just defining your own custom traversal that also opportunistically compiles.

Also, FYI, I added a method to make it easier to get options from the command-line: https://github.com/cucapra/calyx/pull/916/files#diff-6fb447740ea6c13bca4b0f91c7895c00bc01be804f4e3eb7f967498e2c596362R604
Feel free to update the code to use it if you'd like!

So, the control.borrow_mut() option will just give you mutable access to the whole control program:
https://github.com/cucapra/calyx/blob/master/calyx/src/passes/top_down_compile_control.rs#L642
Your traversal can just muck with the pointers as it pleases. Note that this is only possible in the start or finish methods because otherwise, the visitor owns the control tree (and is allowed to modify it).
During a bottom-up recursive traversal, you can keep track if the current level of control nodes can be statically compiled and keep popping up till you find a node that cannot be compiled.
When you reach this node, your traversal can construct the new group and the current parent (which cannot be statically compiled), can replace the pointers as needed.
Does that make sense?

Alright, I pushed up a rough draft. I think this works how we want and is much nicer than the alternatives. Two things I want to address:

share the common structure of all the start_* methods, I just copy pasted for now
remove the optionality from the calculate_states functions, and instead assert they only deal with static control

Got it, the cycle count updates in frontend correctness tests seems fine because they are supposed to be end-to-end tests with the default compilation pipeline enabled.
I'm not sure exactly what would be going wrong but you could try disabling the optimization passes and seeing if that changes anything. Also worth running with the interpreter to see if the answer is different (which would indicate a lowering bug somewhere):
fud e --to interpreter-out -s verilog.data <data file> <input file>

The optimization passes used is defined here: https://github.com/cucapra/calyx/blob/master/calyx/src/default_passes.rs#L100
You can selectively disable optimizations defined in the pre-opt and post-opt aliases and see if that fixes anything.

I see the correct results from the interpreter. If I disable just top-down-st, I see correct results from the compiler. I will try keeping top-down-st and disabling the opt passes to see if there's any difference.

You can also run the interpreter after running top-down-st:
fud e --to interpreter-out ... -s futil.flags ' -p validate -p infer-static-timing -p top-down-st'

This has the effect of running the Calyx compiler on the input program first, and then passing the new program to the interpreter. If the interpreter also says that the output changes after compilation, something is wrong. The benefit of doing this is that it removes the cofounding factor of compiling all the way to RTL–the interpreter will run the program right after top-down-st is done.
Also, fud pro-tip: you can use the -n flag to print out what commands are going to run (dry run mode). Often time, we'd have setup things in a way such that you can run the Calyx compiler before running another tool like the interpreter.

You can also run the interpreter after running top-down-st

Thanks for the tip. I am trying that approach, but getting errors that groups are unused (using just -p validate -p infer-static-timing -p top-down-st). Maybe that's because after top-down-st there are groups that are no longer mentioned in the control program, and only get handled within the generated tdst groups? I will keep trying to test this angle, but I fear there is actually something wrong with what top-down-st does.

Ah, that's the well-formed pass being overzealous. You are right about why the error occurs and well-formed to consider groups that are structurally mentioned as being used.

Well, I'm stumped and I'm going to have to come back to this later. I did find one bit of info from the interpreter: due to the issue above, I can't have it interpret the IR right after top-down-st, but I did just run the whole lowering:
fud e --from ntt -s verilog.data tests/correctness/ntt-pipeline/ntt-8.txt.data tests/correctness/ntt-pipeline/ntt-8.txt --to interpreter-out

That produces correct results, whereas running the compiler and verilator does not:
fud e --from ntt -s verilog.data tests/correctness/ntt-pipeline/ntt-8.txt.data tests/correctness/ntt-pipeline/ntt-8.txt --to dat

Hopefully this discrepancy can help pinpoint the issue, but I will need to look over the weekend.

Okay, I'll also try to investigate on my end if I get some time. In the meantime, here is a set of tips for debugging Calyx we previously wrote up in case they are helpful: https://docs.calyxir.org/debug/index.html

Okay, so to debug, I used the ./tools.flag-compare script and used the par-to-seq pass to transform all the par blocks into equivalent seq blocks which removes the problem. This implies that something is wrong with the par encoding. I'm going to try to minimize the program now.

ntt-8.futil.txt
Here is a minimized Calyx file that uses the same input as the above file and generates different results with top-down-st enabled. Only the par block in the control program is compiled with static timing so something is wrong with it. I'm going to go stare at some VCDs.

used the par-to-seq pass to transform all the par blocks into equivalent seq blocks which removes the problem.

I tried this, and what I saw was the par nodes were made into seq nodes, and a later pass inlined them so there is one flat seq node with mixed static and non-static enables. That effectively disabled the top-down-st pass.

Here is a minimized Calyx file that uses the same input as the above file and generates different results with top-down-st enabled. Only the par block in the control program is compiled with static timing so something is wrong with it. I'm going to go stare at some VCDs.

Thanks for trying to find a minimal reproducer. I can try to debug this as well.

Seems like the multiplier's inputs are cut-off prematurely causing it to output a zero.

Hmm, isn't this multiplier pipelined? I will go check the implementation, but I guess we need to take the initiation interval into account?

Yeah, not sure what's going on but maybe the static annotation for the multiplier is wrong? The latency should be 3 cycles (so that they correctly get mapped on DSPs on Xilinx boards) but maybe our implementation is wrong.

Argh, the multiplier implementation is to blame here. The implementation takes 4 cycles instead of 3. I'll fix it.

oh geez. well, good to get to the bottom of that i guess.

Okay, if you merge changes form #905, things NTT tests should pass.

Thanks for the help @rachitnigam, it looks good to me now. The only difference for the ntt tests is it takes less cycles, as we saw for the others. I'll put in a couple cleanups and this should be ready for a serious review.

seq transition logic can be simplified (question from the original PR)

I'm okay with the seq logic as it is. I think there needs to be some redundancy with that logic somewhere; tdcc does something similar with enables I think.

@rachitnigam I think this should be good for review, the housekeeping items should be addressed. What's the best way to reduce duplication in the start_* methods? I tried to write a generic version that just called "calculate_states", but I'm not sure how to get the attributes out of a generic ir::Control.

You can use the GetAttributes trait which is implemented by Control nodes.

Sorry for the noob question, but I am trying to write a generic function that takes a &mut ir::Control, and pass it a &mut ir::Seq, for example. How do you call such a function? It looks like:
fn start_generic(
    con: &mut ir::Control,
    comp: &mut ir::Component,
    sigs: &LibrarySignatures,
    dump_fsm: bool,
) -> VisResult {
...

And I want to say something like:
    fn start_seq(
	&mut self,
        con: &mut ir::Seq,
        comp: &mut ir::Component,
	sigs: &LibrarySignatures,
        _comps: &[ir::Component],
    ) -> VisResult {
        start_generic(con, comp, sigs, self.dump_fsm)
    }

This doesn't work, so I'm trying to "upcast" the &mut ir::Seq to a &mut ir::Control. I tried something like this, but that tries to move out of a mutable reference:
start_generic(&mut ir::Control::Seq(*con), comp, sigs, self.dump_fsm)

Hm, generally you’d have to do this by defining start generic on some shared trait implemented by all the control nodes but there isn’t such a thing. Additionally, since you’re going to dispatch on the input anyways, it might not be useful after all.
maybe it’ll help if you explain which boilerplate you’re trying to remove and I can try to explain a Rust-y way to do it.

Well, all of the start_seq, start_par, etc. all have the same structure, but dispatch to the specific _calculate_states methods. I was thinking it would be good to write a single method with the same structure, which calls the generic calculate_states. start_seq and friends could just call the generic method with their specific control construct.

I see, I'd probably separate the checking logic into a function that takes something that implements GetAttributes:
fn check_static<C: GetAttributes>(con: C) -> bool {
  ...
}

I don't currently have an obvious way to take something like this and also dispatch it. One probable way is using dynamic trait objects (dyn impl) but that forces Rust to build a virtual table for the function which might be unnecessary in this case.

I don't currently have an obvious way to take something like this and also dispatch it.

If you're fine with this level of duplication, so am I. The checking could be factored out, but it's not going to save much.

I've felt a comment on a simple FSM optimization that could be useful but I'll leave it to your judgement

I was thinking about this a bit as well. In a lot of cases I think we would be able to have the FSM state simply increment.
I'm inclined to leave this be for now, and consider this a starting point for more optimizations. This currently provides the behavior I'm looking for, so I'm happy if further resource reductions are added later, along with #911 and other such optimizations.

Thanks for all the iterations on this @rachitnigam! Landing this now as a starting point, and we can tackle the further enhancements in smaller PRs now that the functionality is locked in.


Cleans up deprecated APIs

This PR branch is based on databricks/spark-redshift#221 and contains its changes. For easier review, please refer to https://github.com/liancheng/spark-redshift/pull/1.
This PR cleans up all the deprecated APIs in 1.x:

Parameters.overwrite
Users should use SaveMode instead.

SchemaParser
Removed since the only API method uses this class is removed.

RedshiftDataFrame.saveAsRedshiftTable(parameters: Map[String, String])
Users should use DataFrameWriter methods instead.

RedshiftContext.redshiftTable(parameters: Map[String, String])
Users should use DataFrameReader methods instead.

RedshiftContext.redshiftFile(path: String, schema: String)
Removed since we'd like to stop using string schema parsing. The following API method is added to replace it:
def redshiftFile(path: String, schema: StructType): DataFrame

Fixes #66.

@JoshRosen Since the following public method in RedshiftContext isn't marked as deprecated
def redshiftFile(path: String, columns: Seq[String]): DataFrame

I added a new version of redshiftFile()
def redshiftFile(path: String, schema: StructType): DataFrame

to replace the deprecated one:
def redshiftFile(path: String, schema: String): DataFrame

These methods are used to read data files unloaded from a Redshift table using RedshiftInputFormat, and can't be replaced by existing DataFrame reader/writer API. Another choice can be to make  RedshiftRelation support reading unloaded files, so that we can have a more unified API. What do you think?

[Current coverage][cc-pull] is 76.21%

Merging [#239][cc-pull] into [master][cc-base-branch] will decrease coverage by 13.21%

@@             master       #239   diff @@
==========================================
  Files            13         12     -1
  Lines           681        639    -42
  Methods         596        557    -39
  Messages          0          0
  Branches         85         82     -3
==========================================
- Hits            609        487   -122
- Misses           72        152    +80
  Partials          0          0

Powered by Codecov. Last updated by [a1749cb...768bf99][cc-compare]
[cc-base-branch]: https://codecov.io/gh/databricks/spark-redshift/branch/master?src=pr
[cc-compare]: https://codecov.io/gh/databricks/spark-redshift/compare/a1749cb8dd3b064a930c19f8cd12e557a274b1aa...768bf990b39ed27b91b201071c6deb303248c374?src=pr
[cc-pull]: https://codecov.io/gh/databricks/spark-redshift/pull/239?src=pr

@liancheng, in the long run I think it would make sense to open up more modularized APIs to the spark-redshift internals in order to address advanced use-cases (such as the one in #197), so at some point I think we'll want to have a separate component for reading unloaded files. I don't think that we necessarily want to expose that functionality publically through RedshiftRelation since it's not the easiest class to construct AFAIK.
Regarding the individual deprecations:

+1 on removing Parameters.overwrite; this is an obvious good chose.
+1 on removing saveAsRedshiftTable and redshiftTable, since these are subsumed by the data sources API.
+1 on replacing redshiftFile(path: String, schema: String) by redshiftFile(path: String, schema: StructType), but I think that this introduces a small gap in functionality w.r.t the old code since I don't think spark-redshift exposes a great API to retrieve the schema of a Redshift table independent of unloading / reading that table, so it might be a little tricky for the user to actually obtain the schema object. We could try to address this by opening up an API for calling the schema-retrieval code over JDBC, but I don't think that is quite a sufficient solution since the old schemaString approach could be used in cases where the Spark cluster could not connect to Redshift / didn't require a JDBC connection. For this reason it might be nice to expose the SchemaParser object so that a user can call that as a shortcut helper method for constructing the Spark Schema object. However, the current SchemaParser is slightly painful to use from Java. Therefore, I think it might make sense to expose a public SchemaParser class in Java with static method and keep the parser implementation in a private Scala helper class that it calls.

I don't want to overengineer things now, though, so I'd be fine with rebasing and merging these changes as-is and dealing with feature-gaps / requests in followups or in a followup to the 2.x release.

Yeah, so TL;DR: rebase this and I'll merge it.

@JoshRosen Thanks! Having a Java version of SchemaParser sounds good to me. Finished rebasing.

LGTM so I'm going to merge this to master. Thanks @liancheng!


Proof keys: add details on URL escaping

For the purposes of calculating proof keys, the URL must be aggressively escaped. All non-alphanumeric characters (that is, those outside the range [a-zA-Z0-9]) must be URL-encoded.

Clarification: this only really applies to the access_token on the URL.


CT entries are removed/reset every 5 minutes

Verify what will be the outcome in the CT by running #2684 while policies are re-fetched from k8s.

Nothing is changing in the policy, this regeneration is being triggered by this periodic trigger so the CT table should not be affected.
 If a connection has not received traffic in the last 5 minutes, we will delete the connection. In this case, if the application sends another packet after this period, we will simply drop the packet. This results in a "stuck" connection. We don't currently attempt to reset the connection.
To avoid connections getting stuck, we can either:

Make the application send keepalives once every 5 minutes, or
Send a reset in response to a packet that gets dropped and assume the application will reopen the socket, or
Increase the timeout

Increase the timeout, or

Let's increase it to 2 hours.

Send a reset in response to a packet that gets dropped and assume the application will reopen the socket, or

I think this will cause more problems than it solves

Pick up connections in the middle

I'm open to this. We discussed this earlier. There is no point in protecting the TCP stack so we just replace the logic that currently requires a SYN for a conntrack create and also remove the logic that deletes on RST.

I will give this a shot, this has been bothering me long enough ;-)


connecting to wrong sts endpoint https://sts..amazonaws.com

Describe the bug
I'm using your helm chart and I have passed the assume role PROMETHEUS_RDS_EXPORTER_AWS_ASSUME_ROLE_ARN: arn:aws:iam::00000000:role/rds-exporter and I see it is trying to get the GetCallerIdentity it is trying with wrong sts end point. below is the log
{"time":"2023-12-27T18:17:04.653877227Z","level":"DEBUG","msg":"Assume role","role":"arn:aws:iam::00000000:role/rds-exporter"}
{"time":"2023-12-27T18:17:04.653954968Z","level":"DEBUG","msg":"search AWS region using IMDS"}
{"time":"2023-12-27T18:17:04.656602818Z","level":"INFO","msg":"found AWS region via IMDS","region":"us-east-1"}
{"time":"2023-12-27T18:17:04.658949862Z","level":"ERROR","msg":"can't identify AWS account and/or region","reason":"can't fetch information about current session: operation error STS: GetCallerIdentity, get identity: get credentials: failed to refresh cached credentials, operation error STS: AssumeRole, https response error StatusCode: 0, RequestID: , request send failed, Post "https://sts..amazonaws.com/": dial tcp: lookup sts..amazonaws.com: no such host"}
A clear and concise description of what the bug is.
wrong sts end point
Desktop (please complete the following information):

OS: [e.g. Linux]
Prometheus RDS exporter's version [e.g. 1.1]
RDS engine: [e.g. PostgreSQL]

To Reproduce
add env parametes as below in the vaules.yaml
env:
PROMETHEUS_RDS_EXPORTER_DEBUG: "true"
PROMETHEUS_RDS_EXPORTER_AWS_ASSUME_ROLE_ARN: arn:aws:iam::00000000:role/rds-exporter
PROMETHEUS_RDS_EXPORTER_AWS_ASSUME_ROLE_SESSION: "rds-exporter"
Steps to reproduce the behavior
Expected behavior
A clear and concise description of what you expected to happen.
It has to connect to https://sts.amazonaws.com
Additional context
sts endpoint has 2 dots (..)
Add any other context about the problem here.

Hello,
There is a problem finding the correct address for the AWS STS service. It should be sts.us-east-1.amazonaws.com (or sts.amazonaws.com in legacy environment).
It could be related to the AWS authentication method used. I'd be happy to help you solve this deployment problem.
Can you share more context about your deployment:

What version of the RDS exporter do you use?
Are you using AWS EKS?
What authentication method are you using (IRSA, AWS IAM credentials via AWS environment variables, AWS IAM credentials in a file, SSO, ...)?

And test the following steps:

Check that the AWS credentials used in the Kubernetes pod are correct
Set AWS_DEFAULT_REGION and AWS_REGION

Thanks,

Hello @Sampathreddygopireddy118,
Do you still have an issue with STS? If yes, can you apply suggestions from my previous message. If there is no update, I will close the issue by the end of the week to avoid outdated issues.
Thanks,

the issue will also occur when fetch metrics from AWS China, since the domain name for AWS China is amazonaws.com.cn rather then amazonaws.com

The AWS SDK should use amazonaws.com.cn domain automatically. To make it work you need to set AWS_DEFAULT_REGION and AWS_REGION environment variables and  the role ARN should use aws-cn partition like arn:aws-cn:iam::00000000:role/rds-exporter
Do you still have the issue with these parameters?

Closing this request as it looks like it has been resolved with last comment.


Gif on readme is out of sync

Please fix this, it doesn't accurately represent the capabilities of your system! I would love to see this synced up!!!

Agreed!

Very disappointing that the gifs are not synchronized. I was about to use this project for my multinational enterprise, but upon seeing this I have henceforth changed my mind.
If this is fixed, I will reconsider this.

I am so sorry to hear about your situation. I hope your large multinational enterprise business was not disrupted too greatly. Warmest wishes, Arnav.

Looks like there's not much traction on this issue, closing for now.


Do the traffic lights automatically change? If not, please add

Traffic lights can change now(every 30sec).
In the next version, the traffic lights need to be linked to the traffic control box.


Ensure correct handling of keytype oct with k being bigger than the symmetric algs keysize

Just tested, this works:
#[test]
fn smoke() {
    use ::hmac::{Hmac, Mac};
    use sha2::Sha256;
    let key = b"A";
    let mut mac = Hmac::<Sha256>::new_from_slice(&key[..]).unwrap();

    mac.update(b"Input");
    std::println!("{:x}", mac.finalize().into_bytes())
}

... and does not conform to RFC 7518 section 3.2

A key of the same size as the hash output (for instance, 256 bits for
"HS256") or larger MUST be used with this algorithm

We need to add a check for the correct key length


[Proposal] 15: Split Coupon Underlying

[Proposal 15]: Split Coupon Underlying
Summary

Implements EIP-16

Description
EIP-16
EIP-16 aims to significantly reduce the risk of coupons by splitting their principle with their premium reward, treating them differently upon expiration.
Expiration Logic

Underlying (principle) is locked until either:

the premium can be paid out of the redeemable pool or
the coupon has expired

Expired coupons no longer have the ability to claim their premium reward, but can claim their underlying.

Economic Specification

Locked ESD is burned on coupon purchase and minted on underlying redemption so that it is programmatically discluded from total supply.
Redeemable ESD is only needed to cover the premium reward, principle is simply minted fresh.

Migration
A new migrateCoupons method is exposed for current coupon holders. This allows current coupon holders to do a one-time migration (per-epoch) to split their coupons 50/50 between underlying and premium. We chose 50% as it is strictly better from a risk perspective than currently outstanding coupons, but it still worse than  a post-EIP-16 purchase, so there isn't an opportunity to game purchases ahead of this proposal.
Debt will be reset to 0 since this will significantly effect the risk profile of coupons, and we can't accurately predict how this will shift the market premium.
QoL Improvements

Reduces the emergency commit quorum from 66% to 40% inline with the previous 33% to 20% quorum reduction.
Increases advance reward from 100 ESD to 1000 ESD to counteract rising ETH and gas prices.

Rewards

Rewards committer with 1000 ESD
Rewards @sabretooth with 5000 ESD

Tracking
Status: Pre-proposal
Implementation:

How does this change decrease the supply when ESD is essentially not burned at all, but rather converted to coupons for 30 days?

How does this change decrease the supply when ESD is essentially not burned at all, but rather converted to coupons for 30 days?

Contract Deployed:
Address: 0x9ac39b3e72f3bc07520355a9be5398656c8bc514
Diff Checker: here

Contract Deployed:
Address: 0x9ac39b3e72f3bc07520355a9be5398656c8bc514
Diff Checker: here


feat: with clauses for congr!/convert/convert_to

Adds with clauses to congr! and friends consisting of a list of rintro patterns. These patterns are used when these tactics intro variables.

bors merge


Added "nullable" constraint and fixed enum+text usage

Added new directive, tests and usage examples. Also fixed Issue#113
The idea is that some key could be not required, but if it is present in the schema it must not be empty.

Coverage increased (+0.1%) to 87.303% when pulling 51cfb58216ad31ecf4bce2c0f74e217ce8873819 on Mallone63:nul_and_text_enum_fix into e7fac6a5f5d27a472b206b896f684ee83650b5be on Grokzen:master.

Coverage increased (+0.1%) to 87.303% when pulling 51cfb58216ad31ecf4bce2c0f74e217ce8873819 on Mallone63:nul_and_text_enum_fix into e7fac6a5f5d27a472b206b896f684ee83650b5be on Grokzen:master.

@Mallone63 Very good contribution, you covered all checks and requirements for merge :] Many thanks for this contribution.


discard_pre and post

For certain data acquisitions (e.g. spiral, see example here: https://github.com/ismrmrd/ismrmrd/blob/b320375e0964366d41325574bcc7f0f15a0833bb/examples/matlab/simple_spiral_recon.m#L29) not all readout points are meant for image reconstruction. The points to skip at the beginning and end of each readout is specified by discard_pre and discard_post, respectively.
Currently we read out these parameters but we do not use them to select the imaging data.

This is still a todo.
If we handle it already when reading in the KData, we can remove the discard_pre and discard_post parameters from the acqinfo later on -- and don't have to deal with them in rearange/indexing

Can we assume that discard_pre and discard_post are constant?
or at least, can we assume that n_samples - discard_pre - discard_post is constant?
otherwise we could either

cut to the smallest size among all acquisitions or
keep them and only set the trajectory to nan for points we want to ignore.
the latter would require a modification in FourierOp and CartesiaSamplingOp as well.

@ckolbPTB @schuenke


LWJGL not in classpath

When running the 1.3.1 release as a slave:
@15:53:16.380           Main thread  INFO: Starting Visage v1.3.1 as a slave
@15:53:16.607           Main thread  INFO: Slave name is '55c4ee1bdac1'
@15:53:16.612           Main thread  INFO: Press Ctrl+C to shutdown Visage.
@15:53:16.616          Slave thread  INFO: Setting up LWJGL
Exception in thread "Slave thread" java.lang.UnsatisfiedLinkError: no lwjgl64 in java.library.path
at java.lang.ClassLoader.loadLibrary(ClassLoader.java:1889)
at java.lang.Runtime.loadLibrary0(Runtime.java:849)
at java.lang.System.loadLibrary(System.java:1088)
at org.lwjgl.Sys$1.run(Sys.java:72)
at java.security.AccessController.doPrivileged(Native Method)
at org.lwjgl.Sys.doLoadLibrary(Sys.java:66)
at org.lwjgl.Sys.loadLibrary(Sys.java:87)
at org.lwjgl.Sys.<clinit>(Sys.java:117)
at org.lwjgl.opengl.Pbuffer.<clinit>(Pbuffer.java:148)
at com.surgeplay.visage.slave.VisageSlave.run(VisageSlave.java:92)

Are you using the run.sh or run.bat?

run.sh

Okay, I thought you may have been running it as java -jar directly, hence the question. (Realized it's not entirely clear)
What operating system? I've only tested this on Linux, not Unix or macOS.

I get the same error on both macOS and Ubuntu 16.04

Does your lib/natives directory have a liblwjgl64.so and a liblwjgl64.dylib?

No... I don't see either of those in the zip from the releases page either though

I tried building it w/ Gradle to see if a build I ran was any different, and got this error:
FAILURE: Build failed with an exception.

* Where:
Build file '/Users/robert/projects/Visage/build.gradle' line: 8

* What went wrong:
An exception occurred applying plugin request [id: 'com.stehno.natives', version: '0.2.4']
> Failed to apply plugin [id 'com.stehno.natives']
   > Could not create task of type 'UnpackNativesTask'.

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.

BUILD FAILED

Yeah, I don't have a liblwjgl64.dylib either, but I do have a .so.
What happens if you rename liblwjgl.dylib to liblwjgl64.dylib? (This may sound really dumb, I have no idea how macOS libraries work, but file claims it's a "universal" binary with 64-bit capability)
I don't think the project builds on new versions of Gradle. I need to update it.

I don't seem to have any liblwjgl.*... only lwjgl-*. Here is what I have in the lib directory:
Visage-1.3.1-SNAPSHOT.jar
amqp-client-3.5.1.jar
commons-pool2-2.0.jar
config-1.2.1.jar
gson-2.2.4.jar
guava-18.0.jar
jansi-1.11.jar
javapng-2.1.2.jar
javax.servlet-api-3.1.0.jar
jedis-2.6.2.jar
jetty-http-9.3.0.RC0.jar
jetty-io-9.3.0.RC0.jar
jetty-server-9.3.0.RC0.jar
jetty-util-9.3.0.RC0.jar
jinput-2.0.5.jar
jinput-platform-2.0.5-natives-linux.jar
jinput-platform-2.0.5-natives-osx.jar
jinput-platform-2.0.5-natives-windows.jar
jopt-simple-4.5.jar
jutils-1.0.0.jar
lwjgl-2.9.3.jar
lwjgl-platform-2.9.3-natives-linux.jar
lwjgl-platform-2.9.3-natives-osx.jar
lwjgl-platform-2.9.3-natives-windows.jar
lwjgl_util-2.9.3.jar
mcauthlib-1.0-SNAPSHOT.jar
natives
token-bucket-1.6.jar

lib/natives, not just lib.

facepalm sorry. I have all the files you mentioned:
liblwjgl.dylib
liblwjgl.so
liblwjgl64.so

Also, the service keeps going down because I don't have many slaves and the GPU in my server is borked. I spent a  few hours trying to get X to work properly on the server yesterday and didn't get it quite working, so if after this is fixed you want to join the official slave pool I can give you the credentials.

Okay, as suggested, rename liblwjgl.dylib to liblwjgl64.dylib.

Renamed and the same issue persists

Um... grep 'library' bin/Visage?

DEFAULT_JVM_OPTS='"-Djava.library.path=./lib/natives/"'

Ahh one sec, let me try something

I was running Visage from a different working directory, so that relative path screwed it up. It's fixed now.
Thanks :)

@unascribed I've been trying to get a simple Docker image of Visage working, and I got stuck at this point:
Exception in thread "Slave thread" java.lang.ExceptionInInitializerError
at org.lwjgl.opengl.Pbuffer.createPbuffer(Pbuffer.java:234)
at org.lwjgl.opengl.Pbuffer.<init>(Pbuffer.java:219)
at com.surgeplay.visage.slave.VisageSlave.run(VisageSlave.java:92)
Caused by: java.lang.RuntimeException: org.lwjgl.LWJGLException: Could not open X display connection
at org.lwjgl.opengl.Display.<clinit>(Display.java:141)
... 3 more

After that, I tried apt-get installing xorg and openbox, but got stuck where it asks for keyboard configuration... Is this the same issue you're having with X, or a different one?

It's fairly similar - Visage requires direct rendering and OpenGL to operate. The only way to get it to work is to be on baremetal with a GPU, or a VM with VGA passthru. I'm on the former and getting it to work with no display attached is an ordeal to say the least.


Suggestion: [optional-packages] and [suggested-packages]

I've recently found this tool and it completely changed my workflow, I feel like I have a proper NPN or  PHP's composer for Python with virtualenv management built it. I've really liked that you can specify optional and suggested packages in composer and I wish to suggest that functionality for this really great tool:
The optional packages usually don't require for the project to run or develop but they could enhance the experience mostly for development but some cases to the environment as well. A good example could be: django-debug-toolbar, ipython, the optional packages for djangorestframework. I'm sure there is more but I just only used django in my first 6 month so far :smile:
Suggested packages are a subtype of the optional packages, they just a hook to make a note after you firstly installed the project from the really important options.
Both options behave just like the [dev-packages] but with a different flag.
Thoughts, Ideas, Opinions are welcomed!

Great idea, I need this too for a couple of projects. Example for adding an optional package in a "faster" optional group:
pipenv install --opt-group=faster ujson
Installing standard packages and "faster" optional packages in a fresh environment (after git clone) :
pipenv install --opt-groups=faster,other_group

Thanks for the suggestion! We’ve discussed expanding the Pipfile in the past but I believe we currently want to hold firm to the two sections for simplicity and to guard against feature creep.
Feel free to file this against the Pipfile repository, though they move much slower and aren’t likely to change the spec there either

Closing for now. We may revisit in the future


Geo restricting with AWS Amplify and AWS CloudFront

** Please describe which feature you have a question about? **
Since AWS Amplify already uses CloudFront. Why isn't there an option to restrict web apps under the Amplify console?
** Provide additional details**
Is it even possible? If yes, how do I achieve this? I have an application on AWS Amplify and it's working perfectly. However, I need to geo restrict access to the application!

@arximughal have you tried this: https://docs.aws.amazon.com/amplify/latest/userguide/redirects.html#region-based-redirects

Understanding the requirements better: The ask is to provide a way to restrict access to a certain geography.

Yeah. The problem I was facing was that I wanted to geo-restrict the access to the application deployed via Amplify. I still can't find an option inside AWS Amplify console where I can do just that!

I'm facing this problem too, I need to restrict access for users from some countries
I have this in my Rewrites and redirects rules:
[
    {
        "source": "<*>",
        "target": "/access_denied.html",
        "status": "200",
        "condition": "<MA>"
    },
    {
        "source": "<*>",
        "target": "/access_denied.html",
        "status": "200",
        "condition": "<KE>"
    },
    {
        "source": "<*>",
        "target": "/access_denied.html",
        "status": "200",
        "condition": "<TN>"
    },
    {
        "source": "<*>",
        "target": "/access_denied.html",
        "status": "200",
        "condition": "<NG>"
    },
    {
        "source": "</^[^.]+$|\\.(?!(css|gif|ico|jpg|js|png|txt|svg|woff|ttf|map|json)$)([^.]+$)/>",
        "target": "/index.html",
        "status": "200",
        "condition": null
    },
    {
        "source": "/<*>",
        "target": "/index.html",
        "status": "404",
        "condition": null
    }
]

But it doesn't seem to be working, It works for some countries, for example, if I put US or RU it works, but not fo Morocco for example

Any update on this? We're facing the same issue, we need to restrict certain countries from using the app.

This is a much needed feature - if Amplify allowed exposing most of the configs for the underlying CloudFront distro that would be perfect for a lot of these issues

Hello there! I need to georestrict my Amplify deployment to US only. I'm considering creating a new CloudFront distribution with my custom domain so I can set up the georestriction. I'd need to associate the new CloudFront distribution with the AWS Amplify app somehow... I imagine there's a way to do so using "Origins" tab in the CloudFront distro. Can someone sanity check this approach? Thank you!

@vighnesh1987 this might help: https://docs.aws.amazon.com/amplify/latest/userguide/redirects.html#region-based-redirects
I have yet to discover any other concurrent way to achieve this within the Amplify console.

I figured out how to do this properly. When I deployed my app via Amplify, a Cloudfront distribution was automatically created. I confirmed that my app was loading on the URL of that CloudFront distribution. I believe that this is the underlying CloudFront distro that Amplify is using. This is was something I was able to verify when I saw the CloudFront distro URL inside my "Rewrites and Redirects" settings tab in Amplify.
Then I removed my custom domain from the Amplify domain management settings screen. And ported over the custom domain to Route53 and pointed it to the CloudFront distro. Then I set georestrictions inside the CloudFront distro settings to achieve the georestrictions I wanted. I was able to verify those georestrictions on my custom domain.
tldr; Amplify does expose all of the underlying CloudFront settings however it doesn't clearly link back to the CloudFront distro it's using under the hood (but hints at it inside the Rewrites and Redirects panel)
Major unknown here is whether Amplify guarantees that the CloudFront distro will remain the same across deploys – I hope so!

With the new OFAC law put forth in June this year, and going into effect September 12th, Amplify really does need a way to allow us to block countries (in this case, Russia). Businesses on AWS will be on the hook for providing cloud services to Russia.
@mauerbac Is this something you can fast-track to the Amplify team? It's critical from a legal standpoint.

Just spoke with the team. As mentioned in other GitHub issues, we are working on a proper AWS WAF integration but we don't have an exact timeline for this. I'm hoping by end of year. with WAF you can setup geo restrictions.
in the meantime -- you could create a Redirect Rule using Country Codes. This is already supported today. For example you can create the following rule to redirect users coming from Russia to a blocked.html page.
{
    "source": "/<*>",
    "status": "302",
    "target": "/blocked.html",
    "condition": "<RU>"
}

This site should block you from the USA: https://cmh.fdingler.com/.
I will callout that anything regarding rules and regulations is up to your organization to make that call if it covers that restriction properly. So I'm not 100% sure if this will meet your needs. I think our WAF integration is the easiest solution here, but I know it's not available yet.

Could you try this?
[
  {
    "source": "/blocked.html",
    "status": "200",
    "target": "/blocked.html"
  },
  {
    "condition": "<RU>",
    "source": "/<*>",
    "status": "302",
    "target": "/blocked.html"
  },
  {
    "source": "</^[^.]+$|\\.(?!(css|gif|ico|jpg|js|png|txt|svg|woff|woff2|ttf|map|json|xml|webmanifest)$)([^.]+$)/>",
    "status": "200",
    "target": "/index.html"
  }
]

@mauerbac Unfortunately that didn't work either.

Geo restricting using redirects stopped working very recently. It has worked successfully for years. This week I've had 10 hits to my site from the only country code I have configured to be redirected:

Can you comment on why it stopped working and what will be done to resolve this? (Preferably first class support for Cloudflare WAF rules?)

We have about two weeks until the new US OFAC rules go into effect, and companies can be held liable. I'm glad that one of our apps served up via CloudFront + Lambda can take advantage of CloudFlare's geo blocking, but our Amplify app cannot.
It's not a huge deal, as we can manually code this block into our application. But our tiny dev team is kinda tied up at the moment and it'll be challenging to make it happen.
I'm going to open an AWS support ticket, and I'll report back if I arrive at a solution.

@ffxsam I was able to setup the rules as recommended by @mauerbac and redirect successfully the traffic from the US to blocked.html .

You can try it by accessing this app from the US: https://mainline.dvmbd00x72868.amplifyapp.com

I wonder what could be going on with your app. Is it possible that you have local routing on the web browser (e.g. react router) that is messing up the redirects?

@mbacchi it shouldn't have stopped working. Are you saying that you had a rule that redirected traffic away from your site, but now it doesn't?

@ferdingler So I would have a hard time saying it "stopped working" because I don't know how many requests are actually being made from RU to my site. But it certainly has allowed some requests to get through when that didn't happen in the past. As shown in my 2nd screenshot above the redirect rule exists (sending it to an external site,) and I haven't changed the rule since I created it at some point in the last few years. But I noticed these 10 requests that were from the RU even with this rule in place. It hasn't increased from 10 requests, but something happened where these requests were not in fact redirected.

@ffxsam I was able to setup the rules as recommended by @mauerbac and redirect successfully the traffic from the US to blocked.html .

I'll give it another go! I went to your URL and saw a beer. 😆 So it doesn't seem like it was blocking me at all. I assume I should've gotten a 403 error.

Got it! Thanks for your help, everyone!

@ferdingler Actually it appears to be that the rule is no longer working. Another 6 requests from RU in the last 2 days. Should I open a separate issue?


[Indicator additional size]: adding another size (20px) or creating a few new icons

Had another meeting with Yamit and it included a conversation with her another use case.
When the user chooses his/her availability status it causes a dropdown to open with the existing options.
Right now it looks like this:

Alas, it presents us with a problem:
The options presented are actually the avatar indicators that are existing right now.
This situation makes me wonder should we:

Use in this use case the indicator that exists now in its larger size, and in this case the options will look like this:

Yamit said that she doesn't actually have a problem with the way it looks right now,
but I'm bothered by the different sizes of indicators/icons
OR

Create another indicator size (20px) that will be used only in this case, also in this case there's no need for "audio/video call" option

OR

Create additional icons that look like the indicator statuses that we have already (available, do not disturb, busy, away & offline).

@yinonov
What do you think about how it should be handled?
I am actually inclined to leave it as it is, but I wonder maybe we should consider adding another size only for the sake of size consistency in the side drawer element?

Avatar presence cannot be used outside its own Avatar scope. it's an avatar related interface.
This implementation should be manually authored by consumers by using a condensed badge (which should extend support for icon-only).
alternatively, indicator icon representation can be omitted to simply text as in

but that's a UX decision.

I believe that maybe it can be omitted to just text,
but I got the impression that this is not what she wants.
She wants to have the icon aside text to match the indicator.
So maybe we should consider creating a few new icons that look the same?

can you please share the link to Figma design here?

Voila!
https://www.figma.com/file/FomC4lmRFnAbFbq9vtMi18/VBC-Availability-Status?node-id=205%3A49257

I tried using the badge without text, but what we have right now creates an elliptic shape.

(which should extend support for icon-only)

👆

@yinonov
I created a series of badges that contain icon only.
They are in this branch:
https://www.figma.com/file/r8OYLPmRmDqfLKt6KHdJDB/branch/eiSBjHTMsNrioTbA1Kohru/Vivid-UI-Kit---2.0?node-id=1916%3A23428
Need reviewing and (hopefully) merging

commented in figma

Finished fixing what you wrote in the comment
Do you mind going over it?

The component was created and successfully merged to Vivid UI Kit 2.0:
https://www.figma.com/file/r8OYLPmRmDqfLKt6KHdJDB/Vivid-UI-Kit---2.0?node-id=18139%3A74251
In the meantime light mode only


[Feature]: Add the github action workflow for greeting contributors

Is your feature request related to a problem? Please describe.
The repo needs a greeting workflow
Describe the solution you'd like
The current project lacks an automated action to greet contributors. GitHub Actions can significantly enhance project automation. I propose adding a GitHub Action workflow to greet new pull request (PR) and issue creators, fostering a welcoming environment.
Describe alternatives you've considered
Implement a GitHub Action workflow that triggers on new PRs and issues. The workflow should include a greeting message to acknowledge and appreciate the contributors for their efforts.

Assign me here under iwoc @Avinash905

Thank you for focusing on this section but right now its not mandatory to deal with this
Can you focus on Workflow , we are working on Dashboard , if you eagerly contribution on Dashboard section . We would highly welcome towards this organisation !
NOTE : Focus on Level 2 and Level 3 Iwoc


Make donot_pickle a configuration option

Not sure if core is the best section, but this seems not to be celery specific.

Nice!


Create dependency-review.yml

Codecov Report
Patch has no changes to coverable lines.
:loudspeaker: Thoughts on this report? Let us know!.


Timeline

Basics

[ ] I added a line to changelog.md
[ ] The PR is rebased with current master.
[ ] Details of what you changed are in commit messages.
[ ] References to issues, e.g. close #X, are in the commit messages and changelog.
[ ] The buildserver is happy.

Checklist

[ ] I have installed and I am using pre-commit hooks
[ ] I fully described what my PR does in the documentation
[ ] I fixed all affected documentation
[ ] I fixed the introduction tour
[ ] I wrote migrations in a way that they are compatible with already present data
[ ] I fixed all affected decisions
[ ] I added automated tests or a manual test protocol
[ ] I added code comments, logging, and assertions as appropriate
[ ] I translated all strings visible to the user
[ ] I mentioned every code or binary not directly written or done by me in reuse syntax
[ ] I created left-over issues for things that are still to be done
[ ] Code is conforming to our Architecture
[ ] Code is conforming to our Guidelines
[ ] Code is consistent to our Design Decisions
[ ] Exceptions to any guidelines are documented

Review

[ ] I've tested the code
[ ] I've read through the whole code
[ ] I've read through the whole documentation
[ ] I've checked conformity to guidelines

jenkins build please

jenkins build please

@badnames @absurd-turtle @4ydan @markus2330
The PR contains a first version of the new timeline feature without event representation that can now be reviewed. I just set added and removed events to 0 for now and created some left over issues for known missing features.
@4ydan: currently there is no loading indicator implemented for the new timeline. That means that we currently don't have an visual indicator we can use in the e2e tests. I added a task to https://github.com/ElektraInitiative/PermaplanT/issues/1088 that we have to adapt e2e tests after implementing a loading indicator.

jenkins build please

jenkins build please

@markus2330 @badnames i fixed mentioned issues and added some tests. the code itself didn't change that much, but maybe you can test the behaviour again.

jenkins build please

Thanks for the great reviews :D
@markus2330 i added some new test cases and now also managed to check if the sliders itself are correctly synchronized using unit test.  i think we can add more after implementing the event indicators. I also added the loading indicator next to the current map date as we discussed together so that we can avoid flaky e2e tests.
@Bushuo i learned a lot while processing your suggestions 👍 I added some tasks with possible issues concerning react hooks here:
https://github.com/ElektraInitiative/PermaplanT/issues/1107.
I will ping you i if have questions when addressing this issues.


Added diagnostic setting support

Describe your changes
I added the azurerm_monitor_diagnostic_setting resource in the diagnosticsetting.tf file to let users define one or more diagnostic settings for the Azure OpenAI resource. Likewise, I extended the variables.tf file to include a map to let users define one or more diagnostic settings.
I can extend the tests under the examples folder to create a Log Analytics, storage account, and eventually Event Hub and configure one or more diagnostic settings for the Azure OpenAI resource to collect metrics and logs in these destinations.
Issue number
#000
Checklist before requesting a review

[ ] The pr title can be used to describe what this pr did in CHANGELOG.md file
[ ] I have executed pre-commit on my machine
[ ] I have passed pr-check on my machine

Thanks for your cooperation!

@lonegunmanb please could you review this PR from our colleague @paolosalvatori ?
Please enable the E2E Tests pipeline for this PR. Thanks

Sure, no worries, I usually use the snake naming convention. This time I didn't because I saw that the privateendpoint.tf privateendpoint_data.tf files is not compliant to this convention.

Good catch, thanks @paolosalvatori !

LGTM! WDYT @soferreira @zioproto ?

LGTM :)


Blade extensions showing

When using @role...@endrole in my views, I see the following:
http://d.pr/i/1jxrw.png

Whitespace at the end of file in RolesServiceProvider.php


IE support for ol.source.Raster

This brings in<EMAIL_ADDRESS>which has a workaround for the lack of an ImageData constructor in IE (add your votes here to support its addition: https://wpdev.uservoice.com/forums/257854-microsoft-edge-developer/suggestions/8804656-support-for-the-imagedata-constructor).
For image type operations, workers can return ImageData-like objects (with data, width, and height properties).  On IE, they will be called with the same.  No changes should be required for pixel type operations.

This is awesome, please merge.

Thanks for the review @ahocevar.  The raster examples now work on IE 11 (I didn't test elsewhere).  And I'm happier that the raster source tests are being run in Phantom now (the pixelworks tests run in Chrome, Firefox, and IE 11).

The tests are failing for IE 9 and IE 10

The tests are failing for IE 9 and IE 10

Workers and typed arrays are absent for IE 9.  I don't think there's a good reason to try for a workaround there.  IE 10 is missing Uint8ClampedArray but could be made to work if someone was interested.  I'll put together a pull request that skips tests if Uint8ClampedArray is missing.

I'll put together a pull request that skips tests if Uint8ClampedArray is missing.

perfect, thanks

@fredj see #4962.  I only have access to a machine with IE 11 right now.  If anyone with 10 and 9 could help test there, that would be great.
Aside: are we really saying we support IE 9?

Aside: are we really saying we support IE 9?

Maybe we could say we ditch it at 5%?

Sad enough; we still have certain customers needing that support. If we ditched IE 9, we would be locked to that last ol3 version still supporting it... which would basically add pressure to upgrade. That said, I am not blocking to drop support. It'd be nice to know it one point release before we actually drop it.

Good to know @marcjansen.  Doesn't seem like we're doing a ton of extra work for IE 9.  But it also seems like it is going away fast.

not so fast in government agencies ;-)

not so fast in government agencies ;-)

same here ...


not working

bro it is not working in my vs code tried to much but doesnt works
Please help...

I was using it with no problem .. and now it doesn't work


Printing docs on chrome

e.g. https://influxdb.com/docs/v0.9/query_language/data_exploration.html
Chrome displays OK but printing puts the left hand frame all over the middle.
Prints OK on new IE because it doesn't show the left pane at all

@mintyc closing in favor of docs issue https://github.com/influxdb/influxdb.com/issues/392


Null sub-struct fields

Hello, thank you for making this library, it has helped me a lot!
However, I've stumbled across an edge case I believe.
When I scan into a struct that has a pointer sub-struct, it gives an error if the fields of the sub-struct are null. Just to clarify, the reason I have assigned the sub-struct to be a pointer value, is to be able to accept null values. So its <nil> when any of its values cannot be scanned
I haven't looked into it to figure out why, but JSON unmarshal works this way?
Take the following example:
err = pgxscan.Select(ctx, db, &users, `
	SELECT
		a."ignore_field",
		a."address_id",
		b."ignore_field" as "address.ignore_field" -- specifically this line.
	FROM "user" a
	LEFT JOIN "address" b ON b."address_id" = a."address_id"
	`)

See, "address" in this case is pointer to struct (*Address). But when it's fields are NULL, this query returns an error: eg cannot scan null into *string.
I found a workaround using json objects, but it's not particularly tasteful:
err = pgxscan.Select(ctx, db, &users, `
	SELECT
		a."ignore_field",
		a."address_id",
		-- using JSON works fine but needs json tag for each struct field.
		json_build_object(
			'ignore_field', b."ignore_field"
		) as "address"
	FROM "user" a
	LEFT JOIN "address" b ON b."address_id" = a."address_id"
	`)

Is there something I missed, or maybe a better design where I can keep the normal select fields without using JSON?

See a full code example
package main

import (
	"context"
	"fmt"
	"log"

	"github.com/jackc/pgx/v4/pgxpool"

	"github.com/georgysavva/scany/pgxscan"
)

type User struct {
	IgnoreField string   `db:"ignore_field"`
	AddressID   *uint64  `db:"address_id"`
	Address     *Address `db:"address"`
}

type Address struct {
	IgnoreField string `db:"ignore_field" json:"ignore_field"`
}

func main() {
	var err error
	ctx := context.Background()
	db, err := pgxpool.Connect(ctx, "connection-string")
	if err != nil {
		log.Fatalf("couldn't connect: %s", err)
	}

	_, err = db.Exec(ctx, `
	create table if not exists "address" (
		"address_id" integer generated always as identity primary key,
		"ignore_field" text not null
	);`)
	if err != nil {
		log.Fatalf("couldn't create address: %s", err)
	}

	_, err = db.Exec(ctx, `
	create table if not exists "user" (
		"user_id" integer generated always as identity primary key,
		"ignore_field" text not null,
		"address_id" integer constraint user_address_address_id_fk references "address"
	);`)
	if err != nil {
		log.Fatalf("couldn't create user: %s", err)
	}

	insertData(ctx, db)

	users := make([]User, 0)

	// Select 1: not working.
	err = pgxscan.Select(ctx, db, &users, `
	SELECT
		a."ignore_field",
		a."address_id",
		b."ignore_field" as "address.ignore_field"
	FROM "user" a
	LEFT JOIN "address" b ON b."address_id" = a."address_id"
	`)
	if err != nil {
		log.Fatalf("couldn't select 1: %s", err)
	}

	// Select 2: working but with json object (plus needs json tags on each field).
	err = pgxscan.Select(ctx, db, &users, `
	SELECT
		a."ignore_field",
		a."address_id",
		json_build_object(
			'ignore_field', b."ignore_field"
		) as "address"
	FROM "user" a
	LEFT JOIN "address" b ON b."address_id" = a."address_id"
	`)
	if err != nil {
		log.Fatalf("couldn't select 2: %s", err)
	}

	for _, user := range users {
		fmt.Printf("result %+v\n", user.Address)
	}
}

func insertData(ctx context.Context, db *pgxpool.Pool) {
	_, err := db.Exec(ctx, `
	insert into "address" ("ignore_field")
	values ('test address 1');`)
	if err != nil {
		log.Fatalf("couldn't insert address: %s", err)
	}

	_, err = db.Exec(ctx, `
	insert into "user" ("ignore_field", "address_id")
	values
		('test 1', 1),
		('test 2', NULL);`)
	if err != nil {
		log.Fatalf("couldn't insert user: %s", err)
	}
}

Hi there! Your intention to use a pointer to support NULL value was current, but you did it in the wrong place.
The NULL value belongs to the IgnoreField field, and hence, the pointer should be there:
type User struct {
	IgnoreField string   `db:"ignore_field"`
	AddressID   *uint64  `db:"address_id"`
	Address     Address `db:"address"` // Here the pointer is not needed. But you can still use it, it doesn't affect the scanning results.
}

type Address struct {
	IgnoreField *string `db:"ignore_field"`
}

Note that, the Address struct is just a container/envelope for scany, and it doesn't matter whether you specify it by pointer or by value.
I hope that helps, let me know if you have more questions.

Closing due to inactivity