Try to resolve addresses with SOCKS proxies too, as SOCKS4 cannot work with unresolved addresses (#1359)

Fix for #1359 when used with Java 9 which fixed SOCKS4 support

OkHttp is the wrong layer for this fix. It belongs in the underlying platform.

Sorry, but I don't quite agree.

We can't resolve IP addresses here because in practice they don’t exist here. If they did, then what the heck is the point of the proxy?

Access control.
One of the typical use-case of SOCKS proxies afair is a firewall that controls who is allowed to connect where-to.
The problem with SOCKS4 (not SOCKS4a) is, that you must resolve the IP address locally.
The whole protocol is only one request and one response while the request is 0x4 0x1 <two byte port> <four byte numerical IP> <0-terminated username>.
There is no way to use SOCKS4 with a hostname.
SOCKS4a added the possibility to specify an invalid IP, namely 0x0 0x0 0x0 <anthing besides 0x0> and then append the 0-terminated hostname after the 0-terminated username.
SOCKS5 also added the possibility to only specify a hostname or to specify an IPv6 address.
But with SOCKS4, the hostname must be resolved on the client side. If only the proxy server can resolve the hostname, then either SOCKS4a or SOCKS5 has to be used.

Imagine a proxy that provides access not to the broader Internet, but to a private internal network. The DNS mappings in that network are not visible to the OkHttpClient host. Heck, they may even contradict what OkHttp sees.

As written above, then SOCKS4 cannot be used in such an environment.
And think about the other way around, what if the client can resolve the hostname to an IP, but the proxy cannot do it?
Also a valid use-case that is supported with my changes, but not without them.

It’s unfortunate that the JDK maintainers got this wrong.

I don't think they did, as the protocol hardly requires an IP being available and if the InetSocketAddress got intentionally created unresolved, who are they to now try to resolve it.

To actually work around it we’d need to build our own complete SOCKS implementation.

Even then you have to resolve the IP address locally to use SOCKS4, as the protocol does not work without as described above.

I don’t want to do that.

To quote yourself from the issue referenced above: We could implement SOCKS in the application layer, which is annoying, but not that annoying.
So you actually do consider doing it on application level if someone makes a PR for it (as you stated in a later comment).

Not really.
In the first naive implementation I did, it would, as the resolution attempt would throw an UnknownHostException.
Fortunately there are tests that cover this case, so I change to catch and rethrow the exception if it is not a SOCKS proxy.
For a SOCKS5 proxy where the client does not know the hostname, the UHE is caught and an unresolved InetSocketAddress created just like before.
So with my changes you have:

SOCKS4 and only client can resolve the hostname => IP is resolved, everything works
SOCKS4 and only proxy can resolve the hostname => unsupported as the protocol cannot handle it
SOCKS4a and only client can resolve the hostname => IP is resolved, everything works
SOCKS4a and only proxy can resolve the hostname => everything would work if the SOCKS socket factory would support SOCKS4a (which the standard JDK does not)
SOCKS5 and only the client can resolve the hostname => IP is resolved, everything works
SOCKS5 and only the proxy can resolve the hostname => hostname is sent, everything works

before it was:

SOCKS4 => unsupported as no resolution attempt is done
SOCKS4a and only client can resolve the hostname => unsupported as no resolution attempt is done
SOCKS4a and only proxy can resolve the hostname => everything would work if the SOCKS socket factory would support SOCKS4a (which the standard JDK does not)
SOCKS5 and only the client can resolve the hostname => unsupported as no resolution attempt is done
SOCKS5 and only the proxy can resolve the hostname => hostname is sent, everything works

The only issue would be if client and proxy disagree on the IP for the hostname, as neither protocol can send both the hostname and the IP.
But you cannot support both, IP resolution on client side and covering that client and proxy may disagree on the IP for a hostname.
The only way would be to have a manual setting, so that the user can decide whether to try resolution on the client or not, but even then I'd recommend to default to do the resolution on the client as I think it is more common that they do not disagree if both can resolve the hostname.
One other way to handle it would be to only try the IP resolution if SOCKS4 is used, but the API for Proxy does not allow to determine which SOCKS version is to be used, unless you cast the proxy to sun.net.SocksProxy which is then bound to only work on JVMs that have this internal class and I thought it is more appropriate to just always try the IP resolution and just fall back to hostname mode if the IP couldn't be resolved.

Lemme restate: For networks like tor there are serious consequences to getting this wrong.
If you do a DNS request for say whistleblowing.onion on a Tor + SOCKS5 proxy, you do not want your destination hostname to leak out with a local DNS request.

Honest question: who uses SOCKS4? Shouldn’t we just encourage these people to get SOCKS5 or JDK9? I don’t want to add code or complexity to OkHttp unless it has a big positive impact.

My view is that at the worst we should expose internal addresses if a SOCKS4 proxy is used.  But private corporate addresses should be leaked like this by default.

Ok, here a new version, how do you like this one?
You now have to explicitly enable the new behavior.

Could you motivate this problem more? Like, how do we instruct app developers when to set this flag?
I have a strong preference for leaving this to a standalone library that does a SOCKS4-friendly SocketFactory.

More background on me being grumpy. I just read a rant about software size & complexity.
http://tonsky.me/blog/disenchantment/
Our job with OkHttp is to be very small and ship what satisfies 95% of the use cases. We leave the ugliest 5% of use cases out on purpose. If you're unlucky enough to need a SOCKS4 proxy in 2018 I don't really want to burden the rest of us with your problems!

Could you motivate this problem more? Like, how do we instruct app developers when to set this flag?

Well, I thought I made it pretty clear in the JavaDoc of the tryToResolveForSocksProxies method: You need to set this option to {@code true} if you either want to use a SOCKS5 proxy that cannot resolve the hostname but you can locally, or if you want to use a SOCKS4 proxy.

I have a strong preference for leaving this to a standalone library that does a SOCKS4-friendly SocketFactory.

It is not only the SOCKS4 that is not working,
but also the SOCKS5 if the proxy cannot resolve the hostname, but you could locally.
Currently you will just get an error response if the proxy cannot resolve the hostname.

If you're unlucky enough to need a SOCKS4 proxy in 2018 I don't really want to burden the rest of us with your problems!

As stated above, it is not only the SOCKS4 that is not working, but also SOCKS5 where the proxy cannot resolve the hostname while the client could.

Well, I don't have any statistics on where SOCKS4 is still used, but it is a rather easy fix and small change as Java 9 does the rest already.
And I also have no statistics on SOCKS5 servers that cannot resolve the hostname as any installation of any SOCKS5 server could suffer from this problem.

Could you get the JDK maintainers to fix this? They know which SOCKS version they’re talking to and can fix it for everyone in one step.

I don't think it is a JRE problem, especially the SOCKS5 one.
For SOCKS4 they could maybe try to resolve the address themselves, but actually then the Dns you set on the OkHttpClient is ignored as the JRE of course cannot use it.
With my patch the configured Dns would be used to resolve the hostname which is what I would expect as user.
Both for SOCKS4 and for SOCKS5 when the proxy cannot resolve the address.

You need to set this option to {@code true} if you either want to use a SOCKS5 proxy that cannot resolve the hostname but you can locally, or if you want to use a SOCKS4 proxy.
But how does the application developer know this? I don’t think how the SOCKS proxy is configured is the application’s concern; instead it’s the concern of the host OS.
What happens today if I’ve configured my OS to use a SOCKS4 proxy? I assume everything is broken, isn’t it? Like, does HttpURLConnection or Apache HTTP client work? I think the right solution to this problem isn’t to punt complexity to the application layer, but instead to get the user to get rid of their broken proxy.

You don't configure your OS to use a proxy, you configure Java to use a proxy.
And why are you keep just talking about SOCKS4?
SOCKS5 is also not working if you have a proxy server that is not able to resolve hostnames, which is a perfectly valid use-case.
An application developer will set this option if he knows that "leaking" names to the DNS server is not a problem.
So basically if someone develops a library that is possibly used for something like Tor, he will either not set this option or provide an option to set this option.
If someone develops a library that is unlikely to be used for something like Tor, he can set this option if he wants to support SOCKS5 proxies that cannot resolve hostnames and SOCKS4 proxies.
If someone develops an application that is not something like Tor, he can set this option if he wants to support SOCKS5 proxies that cannot resolve hostnames and SOCKS4 proxies.
So for example for Javacord which is a discord api wrapper library to build discord bots, I would set this option to true, as it is ok if the discord hostnames "leak" to the DNS server, as probably in almost any application besides a Tor client.

Like, does HttpURLConnection or Apache HTTP client work?

Why shouldn't they?
The JRE supports SOCKS4 and SOCKS5 with resolved hostnames.
The problem is not the JRE.
The problem is that OkHttp explicitly creates an unresolved InetSocketAddress and thereby actively prevents using SOCKS4 or SOCKS5 with resolved hostnames.
So from a users perspective this is either a bug or an intentional rudeness in OkHttp.
Actually you admitted this yourself.
You actively prevent IP resolving to not "leak" the hostname to the DNS.
It is ok if you do something like that if you think this is a security issue, but only if this behavior can be configure, which my PR adds.
And the added logic is not really that fancy.

I think the right solution to this problem isn’t to punt complexity to the application layer, but instead to get the user to get rid of their broken proxy.

The proxy is not broken, both, SOCKS4 and SOCKS5 without access to DNS are perfectly valid use-cases.

Honest question: who uses SOCKS4? Shouldn’t we just encourage these people to get SOCKS5 or JDK9? I don’t want to add code or complexity to OkHttp unless it has a big positive impact.

Many free proxies are still Socks4 proxies (they where probably setup into universities or companies years ago and never modified since then)

You need to set this option to {@code true} if you either want to use a SOCKS5 proxy that cannot resolve the hostname but you can locally, or if you want to use a SOCKS4 proxy.
But how does the application developer know this? I don’t think how the SOCKS proxy is configured is the application’s concern; instead it’s the concern of the host OS.

I really don't understand why this PR, that address a serious issue, has not been accepted yet. It just gives developpers an option to fix issues with some SOCKS4 and SOCKS5 proxies that wont work without it. And as stated, it is an option that needs to be set to true to be turned on. So it won't change the default okHttp behaviour nor break backward compatibility for developpers.  So why block a PR that is so usefull and just add up less than 50 lines of code?

Potentially relevant
https://daniel.haxx.se/blog/2019/04/13/test-servers-for-curl/

@ramsestom does this work for you?
https://gist.github.com/swankjesse/132ecc49bee853c3c335d33b9a95bef7
What are you using SOCKS4 for?

I am using Socks4 in a proxyrotator that rely on free proxy lists (where many Socks proxies are still Socks4).
Did not test your hack of the SocketFactory but anyway it do not solve all issues you can have with valid Socks4 or Socks5 proxies (whereas Vampire's PR does so I implemented his PR in a fork of Okhttp for now and I don't see any valid argument for not accepting this PR officially)


[MOBILE][DASHBOARD] Delete button for comments

The delete button for deleting comments on mobile needs to be reworked.

I would solve it like facebook.
Long press show a context menu.

looks good


Respect the "/max" parameter when starting console apps with defterm enabled

Summary of the Pull Request

Pipe the ShowWindow value through to ConptyConnection
When TerminalPage receives the new connection, it checks the ShowWindow value and maximizes IF there were no other pre-existing tabs (in glomming mode, we don't want to maximize sessions that did not ask for it)

References
#12154
PR Checklist

[ ] Closes #xxx
[x] CLA signed. If not, go over here and sign the CLA
[ ] Tests added/passed
[ ] Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
[ ] Schema updated.
[x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments
This is just a temporary solution until we change our defterm handoff process. Because of the way the process currently works, we have no way of knowing that the connection has requested the window to be maximized until after we have already started a terminal session. This means that we have to manually maximize the window upon receiving the connection, instead of having the session start maximized, as it probably should.
Validation Steps Performed
start /max python with defterm enabled opens up python in a maximized WT window

@msftbot merge this in 10 minutes

Note to future me: We don't need to do the same with /min, because defterm very explicitly rejects handoff for minimized console apps. This is probably for the best! I put an attempt in 66f8b25ec before I forgot that it was filtered long before the Terminal. NOT doing this for /min saves us all sorts of "what happens if start /min cmd tries to glom?" or "what if someone does start /min cmd && start /max cmd and they glom together?".


Tracking user deletion feature does not work for the default file

Describe the issue:

The tracking user deletion feature [1] does not work with the default delete-event.log file in WSO2 Identity Server v5.10.0 as there are no logs being added to it. It works only if you configure a custom CSV file with the following configuration. This needs to be fixed.
[event.default_recorder.user_delete_event]
...
write_to_separate_csv.path = "${carbon.home}/repository/logs/delete-records.csv"
[1] - https://is.docs.wso2.com/en/5.10.0/learn/configuring-users/#tracking-user-deletion-on-deleting-a-user
Environment information:

Product Version: IS 5.10.0
OS: Linux

This issue is being closed due to extended inactivity. Please feel free to reopen it if further attention is needed. Thank you for helping us keep the issue list relevant and focused!


cabal "default-extensions" not passed to ghc during scan

I prefer to add "default-extensions" to my .cabal file so as not to pollute the top of my source files. However, it doesn't seem as if hsdev passes these options to ghc while scanning the source. As an example, Utils.hs requires OverloadedStrings:
$ hsdev check -f src-emubase/Machine/MemorySystem.hs
{"notify":[{"status":"working","progress":null,"name":".../emuwork/src-emubase/emubase.cabal","task":"scanning","type":"path"}]}
{"notify":[{"status":"ok","progress":null,"name":".../emuwork/src-emubase/emubase.cabal","task":"scanning","type":"path"}]}
{"notify":[{"status":"working","progress":null,"name":".../emuwork/src-emubase/Machine/MemorySystem.hs","task":"scanning","type":"module"}]}
{"notify":[{"status":"ok","progress":null,"name":"...y/emuwork/src-emubase/Machine/MemorySystem.hs","task":"scanning","type":"module"}]}
[{"note":{"suggestion":null,"message":"• Couldn't match expected type ‘Text’ with actual type ‘[Char]’\n• In the first argument of ‘append’, namely ‘\"0x\"’\n  In the first argument of ‘(.)’, namely ‘append \"0x\"’\n  In the expression: append \"0x\" . asHex"},"source":{"project":null,"file":".../emuwork/src-emubase/Machine/Utils.hs"},"region":{"to":{"line":27,"column":26},"from":{"line":27,"column":22}},"level":"error"}, ...]

'OverloadedStrings' is specified in the emubase.cabal file in the Library to which this code snippet belongs.
The hsdev check will work if I put {-# LANGUAGE OverloadedStrings #-} at the top of the source. Not quite sure where to look in the source to start figuring out how to debug and propose a patch. Happy to look at the problem and propose a patch if you can give me a bit of clue.

Thanks for your readiness to help.  Fixed and added simple test for it (https://github.com/mvoidex/hsdev/blob/master/tests/Test.hs#L44)
It's not on hackage yet (there's another bug i want to fix before), but you can clone & build.

Thanks! Will give it a try... :-)

Just started to test, ran into "new" issue, which I'm trying to trace down. Sequence of events:
$ cd src-directory
$ hsdev start
Server started at port 4567
$ hsdev complete -f Foo.hs
Missing:

Usage: hsdev complete [-w|--wide] (-f|--file path)
  show completions for input

I did a bit more digging and recovered the log file from SublimeHaskell. For some reason, hsdev gets a lot of these messages:
13/01/17 19:03:40 -0800 ERROR   33/check> Scope leaves with exception: ghc exception: <stderr>: hPutChar: resource vanished (Broken pipe)

Log file attached. Again, point me in a direction and maybe I can diagnose further.

Oh. Wait a second. I'm blind. The log file is telling me that the "stack" command to get the compiler path failed, so hsdev is trying to execute ghc, which will naturally fail on my system. I think I know where to look in your commits.

@mvoidex: Disregard. Moved this thread to another ticket.


Fixes a bug where numa settings were not properly set (not compiled in)

I am still solving issues related to this bug. Please do not merge until finished.

LGTM. Thanks @cramja for spotting and fixing the bug.


Remove wrong url in benchsuite manifest.

This was a copy-paste error when it was added. The documentation field isn't too important here since this is never published.

Thanks you all
@bors r=hi-rustin

:pushpin: Commit<PHONE_NUMBER>f3aeabcd4c169d4e55a9f0660b71a6 has been approved by hi-rustin
It is now in the queue for this repository.

:hourglass: Testing commit<PHONE_NUMBER>f3aeabcd4c169d4e55a9f0660b71a6 with merge 39a972dd5dda9ee02c0c2844ea2734073360f5ca...

:sunny: Test successful - checks-actions
Approved by: hi-rustin
Pushing 39a972dd5dda9ee02c0c2844ea2734073360f5ca to master...


Reorganize package structure

As suggested in #24 this PR reorganizes and renames some files as described in the commit messages.
If there are any suggestions / improvements please let me know :smile:

I am merging this patch, it is straight forward and brings (only) some refinements of the java packages structure. While I like to have well structured java packages and I think the suggested structure makes sense, we should maybe stop making it more granular at some point to avoid ending up with only one class per package :stuck_out_tongue_winking_eye:

@renner  yeah i noticed JsonParser and Result being pretty lonely now in their packages. I'm sure the parser package will have different parsers in the future to support the multiple formats the netapi provides i'm not so sure on the results package though. Maybe will will find a better package structure as the project grows.


Update the UI for the navbar and remove avatar from omnibar

Finishes #147230765
Finishes #147889023

          1 Message

:book:
Nice, more deletions than insertions :red_circle:

  Generated by :no_entry_sign: Danger


Microsoft.WindowsAppRuntime.dll ships with test functions

Describe the bug
Microsoft.WindowsAppRuntime.dll contains test code that serves no purpose to customers.
MddLifetimeManagementTestInitialize
WindowsAppRuntime_SelfContainedTestInitialize

Steps to reproduce the bug
N/A
Expected behavior
No response
Screenshots
No response
NuGet package version
No response
Packaging type
No response
Windows version
No response
IDE
No response
Additional context
No response

ByDesign. These are undocumented, unsupported, small, not harmful merely existing and exist solely for test purposes in ways that required deep internal support to ensure proper testing. This is not unknown in the industry e.g. SQLite's PRAGMA stats

This pragma returns auxiliary information about tables and indices. The returned information is used during testing to help verify that the query planner is operating correctly. The format and meaning of this pragma will likely change from one release to the next. Because of its volatility, the behavior and output format of this pragma are deliberately undocumented.
The intended use of this pragma is only for testing and validation of SQLite. This pragma is subject to change without notice and is not recommended for use by application programs.

Likewise. They're not APIs. Don't use them. Don't call them. Don't expect stability or even existence.
These aren't the APIs you're looking for :P

@DrusTheAxe I'd look the other way if Windows App SDK didn't have a self-contained option. But now that it does, I think we should be taking all efforts to minimize image size, including trimming out useless code. (A self-contained C++ app right now sits around 188MB.) This code still gets hashed by the loader, its symbols and wil error strings are still baked in, folks still have to download these bytes, etc.

I'm sure there's other ways to accomplish the aims - env.vars come to mind, but then you've got stale env-var query and parsing stuff to figure out. Ideally we're able to test real bits and not just debug bits, but I'm definitely sensitive to the size/space concern.

Perf, disk size, resource utilization (memory etc), and other factors are considered. Per SizeBench against a recent Release/x64 build of main

MddBootstrapTestInitialize == 272 bytes

Microsoft.WindowsAppRuntime.Bootstrap.dll == 137,216 bytes

WindowsAppRuntime_SelfContainedTestInitialize == 109 bytes

Microsoft.WindowsAppRuntime.dll == 1,167,360 bytes

A low cost.
I'm not fond of testability in product impactful ways but in certain cases, when necessary, it can be a reasonable price to pay. But it's something we strive to keep to an absolute minimum, for obvious reasons.

A self-contained C++ hello world app with WAS right now sits around 188MB

What language and architecture is that?
TL;DR I'm guessing you're looking at a C# project. A C++ hello world app creates a $(OutputPath) 100M+ but the actual shipping bits are half that. I'd expect self-contained C# pulls in more (e.g. projections). Does that 188MB also include the MsixContent directory? How about the .NETCore runtime?
I don't have a C# app handy at the moment but do have a C++ self-contained app using 1.1-preview1. Taking a closer look...
I created a C++ helloworld-ish app recently in C++ and the x64 Debug directory = 116M. But that seems a poor estimate as it the directory has...

59.3M of files
57 language subdirs for ~1.1M
Manifests dir = 156K
Microsoft.UI.Xaml dir = 3K
A .tlog dir = 59K
MsixContent dir = 56M

That MsixContent dir appears to be a dupe of the parent. $(WindowsAppSDKSelfContained)=true causes the build to unzip the appropriate architecture's framework package's content to the intermediate directory
<MicrosoftWindowsAppSDKMsixContent>$(SelfContainedIntermediateOutputPath)MsixContent</MicrosoftWindowsAppSDKMsixContent>

That's handled in the nuget's build\Microsoft.WindowsAppSDK.SelfContained.targets line 69+
  <Target Name="GetExtractMicrosoftWindowsAppSDKMsixFilesInputs"
      DependsOnTargets="ResolveReferences">
    <PropertyGroup>
      ...
      <MicrosoftWindowsAppSDKMsixContent>$(SelfContainedIntermediateOutputPath)MsixContent</MicrosoftWindowsAppSDKMsixContent>
...
  <Target Name="ExtractMicrosoftWindowsAppSDKMsixFiles"
      Condition="'$(MSBuildProjectExtension)' != '.wapproj'"
      Inputs="@(MicrosoftWindowsAppSDKMsix)"
      Outputs="$(MicrosoftWindowsAppSDKMsixContent)\Microsoft.UI.Xaml.Controls.pri;$(WindowsAppSDKAppxManifest)"
      DependsOnTargets="GetExtractMicrosoftWindowsAppSDKMsixFilesInputs">

    <Unzip
        SourceFiles="@(MicrosoftWindowsAppSDKMsix)"
        DestinationFolder="$(MicrosoftWindowsAppSDKMsixContent)"
        SkipUnchangedFiles="true"
        OverwriteReadOnlyFiles="true" />

That MsixContent directory is just intermediary build goo. The actual bits to ship = ~60M.
Which sounds about right as
unzip -v packages\Microsoft.WindowsAppSDK.1.1.0-preview1\tools\MSIX\win10-x64\Microsoft.WindowsAppRuntime.1.1-preview1.msix

reports the contents are ~57M.
P.S. the lion's share of that's for code (DLLs). Try
unzip -v packages\Microsoft.WindowsAppSDK.1.1.0-preview1\tools\MSIX\win10-x64\Microsoft.WindowsAppRuntime.1.1-preview1.msix *.dll

reports *.dll content uncompressed = ~52M. The resources.pri is 1M, the various reasons 1M+ and some miscellaneous files rounds out the total.


CORS issues when using VimeoVideoPlayer

I have the following code
Center(
                child: VimeoVideoPlayer(
                  url: 'https://vimeo.com/${mediaItem.video_id}',
                ),
              )

This has been working previously, but suddenly this started throwing a CORS error
Access to XMLHttpRequest at 'https://player.vimeo.com/video/353189349/config' from origin 'https://my-sitename.com' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.
No Configurations have been changed.
This is the version i have
vimeo_video_player: ^0.0.9

@CodeNameNinja Please disable web CORS using the steps below, it may fix your error.
Web cors
1- Go to flutter\bin\cache and remove a file named: flutter_tools.stamp
2- Go to flutter\packages\flutter_tools\lib\src\web and open the file chrome.dart.
3- Find '--disable-extensions'
4- Add '--disable-web-security'
Thanks.

This issue is happening in production, and not happening locally, I am using firebase hosting and cloudflare for DNS resolution

Figured out the issue, had to remove the version of main.dart in index.js in the web folder and redeploy to firebase

Re-opened this issue, because the issue is still persisting, I can get the video player to load, but the video doesent load, due to CORS issues, I believe headers need to be added to the Dio.get() Request

I have encountered a similar issue as the one described before. It seems that the proposed solution to disable web CORS worked locally but not in the production environment.

@CodeNameNinja We've added one parameter named Options? dioOptionsForVimeoVideoConfig to add an option to the Dio GET request. There is no need to pass anything in the header for the Vimeo public API to get the video config but you may use this param to pass the headers that you want.
Closing this issue for now feel free to re-open it if it still exists.
Please check it with the latest version. Thanks.

Is there a solution for this issue ? I ran into the same error and unable to resolve it. I use vimeo_video_player: 0.0.15 on a web platform and I allowed the domain in the privacy settings of the video but the config fetch still fail with with the same error Access to XMLHttpRequest at 'https://player.vimeo.com/video/879726513/config' from origin 'https://my-domain.com/' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.


scheduler: do not remove the operator when the step does not finish

Signed-off-by: Shafreeck Sea<EMAIL_ADDRESS>
What problem does this PR solve?
There is a bug introduced by #1652, in some case, like adding peers or adding learners, the step is left unfinished if the peer is in pending state, although the conf version has changed, in these cases, the  operator will be removed because the controller thought someone has  changed the conf version(in fact, it self did). We fix that by checking if the conf version has actually changed by current step, if it is, the operator is not regarded as stale.
What is changed and how it works?
Only check conf version changes when a new step to be executed.
Check List
Tests

Unit test

Codecov Report

Merging #1715 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1715      +/-   ##
==========================================
+ Coverage    76.6%   76.63%   +0.03%
==========================================
  Files         158      158
  Lines       15573    15576       +3
==========================================
+ Hits        11929    11936       +7
+ Misses       2623     2621       -2
+ Partials     1021     1019       -2

Impacted Files
Coverage Δ

server/schedule/operator_controller.go
87.55% <100%> (-1.82%)
:arrow_down:

server/schedule/test_util.go
65.57% <100%> (ø)
:arrow_up:

server/schedule/operator/operator.go
85.5% <100%> (-0.5%)
:arrow_down:

server/tso/tso.go
78.89% <0%> (-2.76%)
:arrow_down:

client/client.go
66.18% <0%> (+0.41%)
:arrow_up:

server/handler.go
50.13% <0%> (+0.52%)
:arrow_up:

server/grpc_service.go
58.13% <0%> (+0.65%)
:arrow_up:

server/region_syncer/client.go
81.01% <0%> (+3.79%)
:arrow_up:

... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14b91e8...8a43885. Read the comment docs.

@Luffbee Only if there are two operator works on one region at the same time. but consider the network, It's possible but the probability is very small. But theepoch of the region will monotonically increase, so as your suggestion may better than now.

@Luffbee I thought about your suggestion before and gave up. From the perspective of code, they really can be merged, but it is weird according to the semantics.  Conf version is not the completion of an epoch. It is acceptable to return if the epoched changed like
func IsFinish(region) (epochChanged bool, finished bool)
It is weird like this
func IsFinish(region) (confVerChanged bool, finished bool)

PTAL @Luffbee @rleungx


[Bug] An open dialogue with allowBackgroundClose set to false prevents access to the window bar

Check the following items

[X] I have looked up relevant Issue

Description of the issue
When creating a dialogue with allowBackgroundClose set to false, it is impossible to move, minimise, maximise, or close the window.
Package Version
6.0.0-beta7
Environment
Win11, GNU/Linux (Gnome on Wayland)
Expected Behavior
The Window's Titlebar should still be clickable and preserve its functionality.
Reproduction
SukiHost.ShowDialog(new ArbitraryDialogueViewModel(), allowBackgroundClose: false);

Additional Information
Dialogues are great to use as an introduction to one's application:

Thus, it would be great if the end user wasn't prevented from moving/closing the application until they finish the intro.

This I think becomes more of a developer implementation issue. The design and functionality of the dialogs are such that they completely dominate the window and trying to "extract" the title bar or add fake close buttons on top of the dialog overlay wouldn't really align with what we were going for with them.
My recommendation would be adding an "Exit" button so that it's crystal clear to the end user that if they exit without setting up the application would close, which would also preserve the design intention of the dialogs.
Alternatively you could use a full window for the setup process which would give you full window-level controls if you're looking for that.

@sirdoombox I understand what you are going for here, and I see value in dominating the window, as to prevent the user from clicking through and accessing other controls. Makes sense. However, I still do not understand the idea on why this would have to apply to the titlebar, ignoring my implementation here. While I can't name one from the top of my head, I've had other applications use a similar thing which still allowed me to move the window around, resize it, etc. I think having that as an option when initialising the dialogue would be nice.
When creating a custom titlebar from scratch, for instance, you have the IsHitTestVisible property.

The ShowDialog methods are modeled after their correspondents from other gui frameworks, such as WPF or Winforms. I agree with @sirdoombox that we should not alter/extend the ShowDialog API for something thats an oversight in your project.
If anything, there should be a new API and terminology for a dialog, that doesnt behave like a dialog, Maybe ShowOverlay could be acceptable?

The dialog host is positioned at the absolute root of the window, the titlebar is contained with in it. In order to enable the titlebar we'd have to pick one of these not very good options:

Make the dialog background click through, with no visual changes this would be confusing and would break a lot of existing behaviour.
Completely reorder the visual tree so the dialog host is within the main body of the window, disabling the ability to make the dialog an absolute requirement and creating a disjointed visual at the top of the window. This would also be a non-optional behaviour.
Re-draw a custom title bar on top of the dialog with a new solid baclground, as the existing one relies on transparency and would look odd being re-drawn above.
Include an optional clear grid space at the very top of the dialog host which can be used to "expose" the title bar - this is probably the least awful of the options but would be a massive headache to implement properly and look terrible as with the top menu enabled the title bar itself has no defined "end". Even worse still when you consider the bottom border can be enabled/disabled.

None of these options really make much sense from our end, especially as you can very, very easily re-implement the dialog host within whatever scope you want if that's the behaviour you're looking for, or as I said you can simply create a new window specifically for your flow or just add a button into the dialog to exit out of the application.
I would seek input from @kikipoulet on this one but I believe we had discussions about the exact way the dialogs would look and feel fairly on and made the call to go with this for simplicity and visual reasons.

Sorry for being late, I'm in vacation. Basically I confirm that the current overlay is on purpose, we do not want to have a title bar visible while showing a modal.
So I think that precise use case shouldn't be included in the library, and for tutorial purpose I would even advice the use of Popups that are more flexible.


No download historical file by Travis

Hi. Thanks for your pull request. Can you please explain what it does? Is it a bug fix or an enhancement? What kind of problem does this solve?

It's is a enhancement.
By default, git takes the whole history of changes in the repo, which is useful when developing code. However, when it is used only as a disk to the latest version is no need to download history. Parameter --depth=1 causes that will be downloaded only the latest version of the application. This speeds up the download of the project, but hinders its development. In this case, the code is not developed, so not appropriate download entirety.

Okay, thanks!


What is the 'rfc-anchor' document identifier?

"rfc-anchor" is not a document identifier format though...

The behavior is as intended. The widget lists every available document type along with sample identifiers. I think the relevant issue was already filed against relaton-data-ids, when fixed this will go away.
By the way, where did you get this screenshot from? I don’t see the latest version deployed on the demo domain.

This issue is now linked to https://github.com/relaton/relaton-ietf/issues/52

Also https://github.com/ietf-ribose/relaton-data-rfcsubseries/issues/3


[@kadena/graph] Alter migrations

Alter migrations to support the correct format for devnet. Also removed unnecessary indexes.
https://app.asana.com/0/1205801361945248/1206475741288313/f

Had to delete the volume to get the new migrations to run successfully. Just making sure this is expected


Only get direct children for loopedslides

We have a situation where we want to have a slider inside a 'looped' slider.
This gives the problem that the events on the nested slider is getting the containing sliders events.
The slider should only look at the direct children to determine what to loop on.

Sorry, i can't merge it. First of all it breaks test scripts. This line doesn't have much sense:
var slides = s.wrapper.children('> .' + s.params.slideClass);

as:

s.wrapper.children - the children method already says to look only in children
'> .' + s.params.slideClass - selector starting from > is not a valid selector for querySelector and it will work only for jQuery


Unclear if DecodeToken() checks for all or any of desiredPermissions

DecodeToken() does not have any documentation stating whether it checks to ensure that all of the desired permissions are present or just any.
My assumption by reading the method signature is that all of the desiredPermissions must be present for this to return nil, however that's not what the code does (which will allow just any matching scope to pass).
ie  a call to uaa.DecodeToken(token, "openid", "cloud_controller.admin") will pass even if only the openid scope is present.
@drnic since I know he's looking at this area...

Hi @aeijdenberg,
We do plan to deprecate this library. You could be getting a token using the oauth2 library and decoding a token can be done with JWT and not using this library.
Sorry that its not a more helpful response.

Hi @adobley - given that deprecation is essentially just a documentation change (ie change the README.md to state "This library is deprecated - please don't use in new projects"), maybe it would be a good idea to bring that forward?
As it stands this looks like a recommended library for talking to UAA - and if the plan is to deprecate it, perhaps the sooner you let people know the better so that less people make it a dependency?

@adobley The diego-release repo also uses this library, so that the route-emitter can send TCP route registrations to the Routing API, so the Diego team would also be interested to know the recommended pathway forward if this library is (pre-?)deprecated.
/cc @jvshahid @goonzoid

@emalm - if you're using this library in prod, you might also be interested in https://github.com/cloudfoundry-incubator/uaa-go-client/issues/15#issuecomment-411601874 which was closed by this team who didn't feel it was important - it's quite lucky JWTs are validated correctly at all in current state.

I think this warrants further discussion since this is being used across teams.
It'd be good to define a clear deprecation path, especially if we're using this library for TCP route registrations.
/cc @rosenhouse @shubhaat @shalako @adobley
Thanks.
Nitya and @utako

We are closing this issue as wontfix and updating the README for this project to indicate that it is no longer maintained.

NOTICE
This library is no longer maintained.
Consider using the cloudfoundry-community/go-uaa client library instead.


Feature/malaviya parth/138

Notes for Reviewers
This PR fixes #138

Changes the Twitter logo to new Twitter X logo
Also has the Custom Scroll-Bar code

Signed commits

[x] Yes, I signed my commits.

Well Again as Preview failed here are the attached screenshots of the output:
Normal

On Hover

Hey @malaviya-parth
Let's discuss it on the websites call.
Please add this as an agenda item in the meeting minutes, if you would :)
https://docs.google.com/document/d/1XczAHXVe2FIWPqiF57ospJ43zw5cZQ7ui8mn39v5EvA/edit#heading=h.lohhtewfwima


tests broken in 2023

running tests
=== RUN   TestGoldenAddCmd
    add_test.go:28: "/build/source/cmd/testproject/cmd/test.go" and "testdata/test.go.golden" are not equal!

        $ diff -u /build/source/cmd/testproject/cmd/test.go testdata/test.go.golden
        --- /build/source/cmd/testproject/cmd/test.go   2023-01-19 16:36:00.335867015 +0000
        +++ testdata/test.go.golden     1970-01-01 00:00:01.000000000 +0000
        @@ -1,5 +1,5 @@
         /*
        -Copyright © 2023 NAME HERE <EMAIL ADDRESS>
        +Copyright © 2022 NAME HERE <EMAIL ADDRESS>

         Licensed under the Apache License, Version 2.0 (the "License");
         you may not use this file except in compliance with the License.

        exit status 1
--- FAIL: TestGoldenAddCmd (0.00s)
=== RUN   TestValidateCmdName
--- PASS: TestValidateCmdName (0.00s)
=== RUN   TestGoldenInitCmd
=== RUN   TestGoldenInitCmd/successfully_creates_a_project_based_on_module
    init_test.go:77: "/build/source/cmd/testproject/main.go" and "testdata/main.go.golden" are not equal!

        $ diff -u /build/source/cmd/testproject/main.go testdata/main.go.golden
        --- /build/source/cmd/testproject/main.go       2023-01-19 16:36:00.515874546 +0000
        +++ testdata/main.go.golden     1970-01-01 00:00:01.000000000 +0000
        @@ -1,5 +1,5 @@
         /*
        -Copyright © 2023 NAME HERE <EMAIL ADDRESS>
        +Copyright © 2022 NAME HERE <EMAIL ADDRESS>

         Licensed under the Apache License, Version 2.0 (the "License");
         you may not use this file except in compliance with the License.

        exit status 1
--- FAIL: TestGoldenInitCmd (0.18s)
    --- FAIL: TestGoldenInitCmd/successfully_creates_a_project_based_on_module (0.18s)
FAIL
FAIL    github.com/spf13/cobra-cli/cmd  0.187s
FAIL

/cc nixpkgs maintainer @ivankovnatsky

Happens every year 🙂.  Add soon as someone posts a PR, I'll merge it


Upgrade terraform provider GitHub to v6.0.0

Fixes #568

Looks sensible aside from the failing diff - looks like a diff in the example generation?

The failing diff was due to a missing pulumi-tls dependency. Should be fixed now.


docs: update Image-Scaner.md

funtionality -> functionality

Title
docs: update Image-Scaner.md

User description
funtionality -> functionality

PR Type
Documentation

Description

Corrected a typographical error in the documentation file Image-Scaner.md, changing "funtionality" to "functionality".

Changes walkthrough 📝
Relevant filesDocumentation

Image-Scaner.mdFix typo in Image-Scaner.md documentation

Projects/2-Intermediate/Image-Scaner.md

Corrected a typographical error in the word "functionality".

+1/-1

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

@CodiumAI-Agent /review

PR Reviewer Guide 🔍

⏱ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No key issues to review

@CodiumAI-Agent /improve

PR Code Suggestions ✨
No code suggestions found for the PR.


entities not recognized

Describe the bug
After installing the 2.00 release the card is not anymore showing as it asks an entities.
Steps to reproduce
No response
Screenshots

This is what i get after applying the breaking changes to the fronted card
Card version
v2.0.0
Integration type
Meteoalarm
Installation type
Home Assistant Community Store (HACS)
Card Configuration
                  - type: custom:meteoalarm-card
                    entities: 'binary_sensor.allarme_meteo_abruzzo'
                    integration: 'meteoalarm'

Integration Configuration
- platform: meteoalarm
  name: 'Allarme Meteo Abruzzo'
  language: 'it'
  country: "italy"
  province: "Abruzzo"

Hey! I am not able to reproduce this issue. While running the same integration and card configuration my card is rendering correctly.

The only solution I can think of is you are using an old version of the card while using the new config. Please check which version of the card you are running by accessing your's browser developer console. It should look something like this.

If it's not 2.0.0 - please update the card if it is, try clearing your browser cache.
Please let me know if it fixes the issue!

I deleted and reinstalled the card, cleared browser cache and the card is now working...
Thanks!

Happy to help!


Added Utf8 validation callback to allow extra validation

Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Codecov Report
Base: 80.79% // Head: 80.74% // Decreases project coverage by -0.06% :warning:

Coverage data is based on head (f06caba) compared to base (022bfb7).
Patch coverage: 75.00% of modified lines in pull request are covered.

:exclamation: Current head f06caba differs from pull request most recent head 7a3d7af. Consider uploading reports for the commit 7a3d7af to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #980      +/-   ##
==========================================
- Coverage   80.79%   80.74%   -0.06%
==========================================
  Files          53       53
  Lines        5665     5660       -5
==========================================
- Hits         4577     4570       -7
- Misses       1088     1090       +2

Impacted Files
Coverage Δ

source/encoding.c
74.68% <75.00%> (-0.21%)
:arrow_down:

source/error.c
72.13% <0.00%> (-5.15%)
:arrow_down:

source/memtrace.c
34.00% <0.00%> (-2.09%)
:arrow_down:

source/bus.c
88.51% <0.00%> (-1.29%)
:arrow_down:

source/xml_parser.c
81.56% <0.00%> (-0.38%)
:arrow_down:

source/array_list.c
77.08% <0.00%> (-0.24%)
:arrow_down:

source/priority_queue.c
90.32% <0.00%> (-0.11%)
:arrow_down:

source/json.c
70.94% <0.00%> (ø)

source/common.c
50.94% <0.00%> (ø)

source/command_line_parser.c
87.14% <0.00%> (+0.18%)
:arrow_up:

... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


nextflow required version

Hi.
Just wanted to clarify pre-reqs.
Based on the intro prereq doc, it said that Install nextflow req was >=20.04.0. Is that a typo or have I accidentally accessed a dev version? It appears >=21.04.0 is required when I try it out. Thanks!

Hi Jacqueline,
Thanks for reaching out! I'll check on the nextflow requirements, but I would strongly recommend using our updated codebase here: https://github.com/refresh-bio/SPLASH . We've worked with some amazing collaborators to make a highly efficient and parallelized implementation, with some more powerful statistical tools. Please let us know if you have any issues with it, or want to discuss how SPLASH could be best be used in your case.
Best,
Tavor

Thanks!


update Lens documentation about additional ways to compose

I just would like to add a note about a few standard composition cases.

Sorry @strobe I somehow lost track of this PR. Thanks for the changes!


Is php-saml vulnerable to the latest SAML authentication bypass vulnerability?

I notice that python-saml has been patched for the CVE-2017-11427 vulnerability(v 2.4.0). Can you please confirm that php-saml is not affected by this flaw?
Reference:

https://kb.cert.org/vuls/id/475445
https://duo.com/blog/duo-finds-saml-vulnerabilities-affecting-multiple-implementations [Duo blog]

Is not affected.
https://github.com/onelogin/php-saml/commit/47cc1092daa416df3188b19966a9870f22d7d456

Do we know if svn php saml  library is affected by the latest SAML authentication bypass vulnerability?

@npadu what do you mean?

We imported our php-saml library from svn

I just wanted to find out if php-saml library is affected by the saml Authentication bypass vulnerability.

I already replied it in this thread...


Interface with spotlight

Spotlight project https://github.com/Renumics/spotlight asked us to interface Nkululeko with their data visualization toolkit.
Seems a very good idea!

i tried their demo at medium<EMAIL_ADDRESS>it seems really easy to interface.
I would suggest to start with a simple flag in the Nkululeko config:
[PLOTS]
spotlight = [train | test | all]
that would cause the spotlight webapp to start-up after having loaded the data and perhaps doing predictions
If that gets more, a dedicated
[SPOTLIGHT]
section of course might also make sense

or, perhaps better in the
section
[EXPLORE]
spotlight = [train | test | all]

Done with version 0.67.0


Fixes #576 Added flags to Singularity Runner

Adds --debug,  --dry-run  and logging functionality to SingularityRunner

Hello @JayjeetAtGithub! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file cli/popper/gha.py:

Line 551:50: E128 continuation line under-indented for visual indent
Line 562:50: E128 continuation line under-indented for visual indent
Line 563:50: E128 continuation line under-indented for visual indent
Line 564:9: E128 continuation line under-indented for visual indent
Line 565:13: E128 continuation line under-indented for visual indent
Line 584:9: E128 continuation line under-indented for visual indent
Line 584:19: E251 unexpected spaces around keyword / parameter equals
Line 584:21: E251 unexpected spaces around keyword / parameter equals
Line 585:9: E128 continuation line under-indented for visual indent
Line 585:13: E251 unexpected spaces around keyword / parameter equals
Line 585:15: E251 unexpected spaces around keyword / parameter equals
Line 587:9: E128 continuation line under-indented for visual indent
Line 588:50: E128 continuation line under-indented for visual indent
Line 589:49: E128 continuation line under-indented for visual indent
Line 590:9: E128 continuation line under-indented for visual indent
Line 591:13: E128 continuation line under-indented for visual indent
Line 591:14: E251 unexpected spaces around keyword / parameter equals
Line 591:16: E251 unexpected spaces around keyword / parameter equals
Line 592:13: E128 continuation line under-indented for visual indent

@ivotron I have added the flags and logging feature. Can you please review upto this stage.Thanks.

@ivotron The travis build stalls after test singularity runs successfully. This does not happen when i run ./singularity in my local machine. Can you please check this ?

@ivotron On debugging, i found that the control is not coming out this loop when the command ./singularity is run through HostRunner. While when ./interrupt or some other tests are run through HostRunner, everything works fine.

 Stopping running actions.
Removing JayjeetAtGithub-actions.simg
(venv3) jayjeet@jayjeet-X541UJ:~/popper/ci/test$ Conrol reached here....
Progress |===================================| 100.0%
Done. Container is at: /tmp/mypaper/singularityhub-centos.simg

But this does not happen when CTRL-C is done manually.

@ivotron Can we change it to a list and check for its length before iterating through the futures?

@ivotron Suppose a kill -2 $pid is ran when the popper run & is running in background and the docker pull <image> step is running, how is the pull process aborted ? Means in which part of the code ?

@ivotron The way we are sending SIGINT in interrupt through kill -2 $pid does not work on background process popper run & correctly. It does not stop the docker and singularity processes that are spawned.  Stopping running actions.
Stopping container 'sleep3'
ERROR: Command 'docker stop -t 1 sleep3' failed: Command 'docker stop -t 1 sleep3' returned non-zero exit status 1.
[sleep4] docker start
[sleep3] docker start
[sleep2] docker start
[sleep5] docker start
++ docker ps -aq --filter status=running
+ test -z '66eaf9855fcb
20fcbab164a5
5ccb5c02a486
93724f294eca'

The above is the behaviour when the sleep is reduced from 20 to 5 in popper run --parallel in docker interrupt tests.
It is completely fine, when popper run --parallel is ran in foreground and CTRL-C is issued.

Thanks a lot for all your efforts Jayjeet. this will likely get solved once #563 gets merged, which should happen soon. docker-py is likely handling this in the same way that singularity but I'm just speculating

@ivotron Thanks a lot! Let's see

Now that #563 is merged, could you please rebase master and see if that fixes the issue?

@ivotron Looks like test parallel got affected after #563. Please have a look !

thanks @JayjeetAtGithub ! Based on this, looks like we are not issuing the signal to the background process appropriately. Can you give that a try? We can also work that on another issue is you prefer

@ivotron i will give a try for sure...

@ivotron Please have a look !

@ivotron Updated the PR. Can't understand why test dry-run failed for python3.7?

@ivotron Fixed it ! Please have a look !

@ivotron I found that docker-py has the facility to stop a container on SIGINT but not remove.

the SIGINT handler only needs to stop containers, not remove them. Removal is done by the runners based on whether the --reuse flag is given to popper run. So if docker-py is correctly stopping containers, then we're good. The ci/test/interrupt test then has to check whether containers are correctly stopped after sending SIGINT

@ivotron The stop_signal option to container.create() that i was trying required relaying the SIGINT to the container from the signal_handler(). That seemed too much unnecessary work and no reduction in code and it was not working properly. Thus i only changed container.remove() to container.stop().

@ivotron Tried some of the features that docker-py provided but nothing worked as required. The above approach seems to be the best to me currently. We can always change it later if something better is found out of docker-py.

thank you so much @JayjeetAtGithub !  thanks a lot for your patience


Added basic HotRod client properties to application.properties

fixes #8

@gytis May I ask you for review?


Bump vcloud-core version to 1.2.0

We need to pull in new cloud-core, which requires
a new fog version, which has functionality we need
in vcloud-edge-gateway.
gds-operations/vcloud-core#177

Superseded by https://github.gds/gds/govuk-provisioning/pull/431/files.

We don't need to make this change to the vcloud-tools metagem, as it will automatically pull in the latest version of vcloud-core on first install.
In the 'govuk-provisioning' repository which we use internally, we have pinned our dependency on vcloud-edge_gateway in our Gemfile, to make sure we have the version we need e.g.:
gem vcloud-tools

# Require at least version 1.5.2 of vcloud-edge_gateway for VPN configuration
gem gem 'vcloud-edge_gateway', '>= 1.5.2'


Apollo and KITTI tracking benchmarking

Hi,
I am benchmarking some results on the Apollo and KITTI tracking datasets linked on this page. I just wanted to confirm the performance IoU is evaluated using the semantic-kitti-api? I.e., generate the labels using the desired MOS algorithm with full-range scans and use evaluate_mos.py directly.
Thank you for sharing these datasets!
Best regards,
Vedant

Are you talking about your own results or the results obtained by using MapMOS? When running our pipeline, you will get a report with the same metrics, just make sure to run it on the full range via the config
mos:
  max_range_mos: -1.0 # No limit

If you want to evaluate your own generated .label results, you can proceed as you described!

Thank you for the prompt response!


[bugfix] Put scripts in comments in Postman import

Description
Bug fix to put scripts in comments in postman import
Contribution Checklist:

[X] The pull request only addresses one issue or adds one feature.
[X] The pull request does not introduce any breaking changes
[X] I have read the contribution guidelines.

This approach would break if the script already has /* */ comments
The safest way is to iterate over each line and just add // in the beginning of each line
Or best of both worlds.
Check if the script has. /* match.
If match found, then // on each line
Else just wrap the whole script like you already have done using /* */
I am fine with either approach

Ok, I will do that tomorrow, I just wanted to do a quick fix, but I agree
with you.
Le lun. 23 oct. 2023, 07 h 25, Anoop M D @.***> a
écrit :

This approach would break if the script already has /* */ comments
The safest way is to iterate over each line and just add // in the
beginning of each line
Or best of both worlds.
Check if the script has. /* match.
If match found, then // on each line
Else just wrap the whole script like you already have done using /* */
I am fine with either approach
—
Reply to this email directly, view it on GitHub
https://github.com/usebruno/bruno/pull/733#issuecomment-1774983719, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AABO3B5OKTISZFTFGOYOSZ3YAZH3VAVCNFSM6AAAAAA6L4MLVWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZUHE4DGNZRHE
.
You are receiving this because you authored the thread.Message ID:
@.***>

Sounds great @survivant !
Merged !!


aws: switch AMI distribution to Ubuntu 20.04LTS

Switch AMI distribution to Ubuntu 20.04LTS.

Oh it's getting error with .github/workflows/build.yml, maybe this need to rewritten to Ubuntu deb based check too.

@syuu1228 @avikivity Perhaps we ought to keep the ability to build RPM-based AMIs too?

@bentsi @fruch I think you are in a better position to review this.

@syuu1228 @avikivity Perhaps we ought to keep the ability to build RPM-based AMIs too?

If it can be done with low effort, it's a good idea.

please remove common/__pycache__/scylla_configure.cpython-38.pyc and common/__pycache__/scylla_configure.cpython-39.pyc from the commits

@syuu1228 @avikivity Perhaps we ought to keep the ability to build RPM-based AMIs too?

I think it would be better to do this work incrementally, first support building a DEB, then support building with DEB.
while still keeping the RPM based builds and AMI building on one of the two.
In a way we could experiment with it, we are not going to do a clear cut like this.

Oh it's getting error with .github/workflows/build.yml, maybe this need to rewritten to Ubuntu deb based check too.

let's add the the needed command to building a DEB package to it, so it would make sure we still build it correctly when there's changes, also as references for changes that would need to take place in the pipelines calling those.

Pushed both distribution supported version.

please remove common/__pycache__/scylla_configure.cpython-38.pyc and common/__pycache__/scylla_configure.cpython-39.pyc from the commits

deleted.

Can you also add the DEB build into the github actions ?

Added.

@avikivity @penberg @fruch Pushed both .rpm/.deb supported version, please review it again

ping

I am going to create the AMI using this code late today and run SCT basic test to validate that the image works.

I am unable to build the AMI
00:33 $ ./build_deb_ami.sh --repo http://downloads.scylladb.com/unstable/scylla/branch-4.4/deb/unified/2021-02-21T16:44:50Z/scylladb-4.4/scylla.list
4.5.dev-20210224.195261c
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   132  100   132    0     0    240      0 --:--:-- --:--:-- --:--:--   240
Hit:1 http://dl.google.com/linux/chrome/deb stable InRelease
...
Fetched 4 109 kB in 21s (195 kB/s)
Reading package lists... Done
W: The repository 'http://ppa.launchpad.net/ozmartian/apps/ubuntu focal Release' does not have a Release file.
N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
N: See apt-secure(8) manpage for repository creation and user configuration details.
W: GPG error: http://downloads.scylladb.com/unstable/scylla/branch-4.4/deb/unified/2021-02-21T16:44:50Z/scylladb-4.4 stable InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 5E08FBD8B5D6EC9C
W: The repository 'http://downloads.scylladb.com/unstable/scylla/branch-4.4/deb/unified/2021-02-21T16:44:50Z/scylladb-4.4 stable InRelease' is not signed.
N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
N: See apt-secure(8) manpage for repository creation and user configuration details.
E: Failed to fetch http://ppa.launchpad.net/ozmartian/apps/ubuntu/dists/focal/main/binary-amd64/Packages  404  Not Found [IP: <IP_ADDRESS> 80]
E: Some index files failed to download. They have been ignored, or old ones used instead.

need to run sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 5e08fbd8b5d6ec9c before running apt update command

another thing, I am not able to build the SMI deb dist/debian/build_deb.sh. Got:
00:59 $ ./build_deb.sh --help
package sudo is not installed
./build_deb.sh: line 21: yum: command not found

after fixing the previous issues I've hit the following issue with starting node exporter:
WARN  2021-02-24 00:42:24,083 [shard 0] iotune - Available space on filesystem at /var/tmp/mnt: 121 MB: is less than recommended: 10 GB
INFO  2021-02-24 00:42:24,083 [shard 0] iotune - /var/tmp/mnt passed sanity checks
This is a supported kernel version.
24 Feb 00:42:32 ntpdate[22784]: adjust time server <IP_ADDRESS> offset 0.000981 sec
Job for scylla-node-exporter.service failed because of unavailable resources or another system error.
See "systemctl status scylla-node-exporter.service" and "journalctl -xe" for details.
Traceback (most recent call last):
  File "/opt/scylladb/scripts/libexec/scylla_setup", line 520, in <module>
    node_exporter_unit.start()
  File "/opt/scylladb/scripts/scylla_util.py", line 779, in start
    return run('systemctl {} start {}'.format(self.ctlparam, self._unit), shell=True, check=True)
  File "/opt/scylladb/python3/lib64/python3.9/subprocess.py", line 524, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command 'systemctl  start scylla-node-exporter' returned non-zero exit status 1.
ubuntu@ip-172-30-0-126:~$ systemctl  start scylla-node-exporter
==== AUTHENTICATING FOR org.freedesktop.systemd1.manage-units ===
Authentication is required to start 'scylla-node-exporter.service'.
Authenticating as: Ubuntu (ubuntu)
Password:
ubuntu@ip-172-30-0-126:~$ sudo systemctl  start scylla-node-exporter
Job for scylla-node-exporter.service failed because of unavailable resources or another system error.
See "systemctl status scylla-node-exporter.service" and "journalctl -xe" for details.
ubuntu@ip-172-30-0-126:~$ systemctl status scylla-node-exporter.service
● scylla-node-exporter.service - Prometheus exporter for machine metrics
     Loaded: loaded (/lib/systemd/system/scylla-node-exporter.service; enabled; vendor preset: enabled)
     Active: failed (Result: resources) since Wed 2021-02-24 00:43:25 UTC; 5s ago

Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: scylla-node-exporter.service: Scheduled restart job, restart counte>
Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: Stopped Prometheus exporter for machine metrics.
Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: scylla-node-exporter.service: Start request repeated too quickly.
Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: scylla-node-exporter.service: Failed with result 'resources'.
Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: Failed to start Prometheus exporter for machine metrics.
lines 1-9/9 (END)

@syuu1228 please run the create image script and debug the issues.

need to run sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 5e08fbd8b5d6ec9c before running apt update command

Added apt-key before running apt-get update.

another thing, I am not able to build the SMI deb dist/debian/build_deb.sh. Got:
00:59 $ ./build_deb.sh --help
package sudo is not installed
./build_deb.sh: line 21: yum: command not found

Added --help message on build_deb.sh.

I am unable to build the AMI
00:33 $ ./build_deb_ami.sh --repo http://downloads.scylladb.com/unstable/scylla/branch-4.4/deb/unified/2021-02-21T16:44:50Z/scylladb-4.4/scylla.list
4.5.dev-20210224.195261c
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   132  100   132    0     0    240      0 --:--:-- --:--:-- --:--:--   240
Hit:1 http://dl.google.com/linux/chrome/deb stable InRelease
...
Fetched 4 109 kB in 21s (195 kB/s)
Reading package lists... Done
W: The repository 'http://ppa.launchpad.net/ozmartian/apps/ubuntu focal Release' does not have a Release file.
N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
N: See apt-secure(8) manpage for repository creation and user configuration details.
W: GPG error: http://downloads.scylladb.com/unstable/scylla/branch-4.4/deb/unified/2021-02-21T16:44:50Z/scylladb-4.4 stable InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 5E08FBD8B5D6EC9C
W: The repository 'http://downloads.scylladb.com/unstable/scylla/branch-4.4/deb/unified/2021-02-21T16:44:50Z/scylladb-4.4 stable InRelease' is not signed.
N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
N: See apt-secure(8) manpage for repository creation and user configuration details.
E: Failed to fetch http://ppa.launchpad.net/ozmartian/apps/ubuntu/dists/focal/main/binary-amd64/Packages  404  Not Found [IP: <IP_ADDRESS> 80]
E: Some index files failed to download. They have been ignored, or old ones used instead.

The error on this console is apt-key problem which I just fixed, but it still not able to succeed since we install scylla-machine-image package from the repo URL, just like we do in CentOS AMI, and the package is not available on released version of deb repo.
To test the option, I created apt repository which including latest master build and scylla-machine-image .deb:
https://scylla-machine-image-test1.s3.amazonaws.com/test.list

after fixing the previous issues I've hit the following issue with starting node exporter:
WARN  2021-02-24 00:42:24,083 [shard 0] iotune - Available space on filesystem at /var/tmp/mnt: 121 MB: is less than recommended: 10 GB
INFO  2021-02-24 00:42:24,083 [shard 0] iotune - /var/tmp/mnt passed sanity checks
This is a supported kernel version.
24 Feb 00:42:32 ntpdate[22784]: adjust time server <IP_ADDRESS> offset 0.000981 sec
Job for scylla-node-exporter.service failed because of unavailable resources or another system error.
See "systemctl status scylla-node-exporter.service" and "journalctl -xe" for details.
Traceback (most recent call last):
  File "/opt/scylladb/scripts/libexec/scylla_setup", line 520, in <module>
    node_exporter_unit.start()
  File "/opt/scylladb/scripts/scylla_util.py", line 779, in start
    return run('systemctl {} start {}'.format(self.ctlparam, self._unit), shell=True, check=True)
  File "/opt/scylladb/python3/lib64/python3.9/subprocess.py", line 524, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command 'systemctl  start scylla-node-exporter' returned non-zero exit status 1.
ubuntu@ip-172-30-0-126:~$ systemctl  start scylla-node-exporter
==== AUTHENTICATING FOR org.freedesktop.systemd1.manage-units ===
Authentication is required to start 'scylla-node-exporter.service'.
Authenticating as: Ubuntu (ubuntu)
Password:
ubuntu@ip-172-30-0-126:~$ sudo systemctl  start scylla-node-exporter
Job for scylla-node-exporter.service failed because of unavailable resources or another system error.
See "systemctl status scylla-node-exporter.service" and "journalctl -xe" for details.
ubuntu@ip-172-30-0-126:~$ systemctl status scylla-node-exporter.service
● scylla-node-exporter.service - Prometheus exporter for machine metrics
     Loaded: loaded (/lib/systemd/system/scylla-node-exporter.service; enabled; vendor preset: enabled)
     Active: failed (Result: resources) since Wed 2021-02-24 00:43:25 UTC; 5s ago

Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: scylla-node-exporter.service: Scheduled restart job, restart counte>
Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: Stopped Prometheus exporter for machine metrics.
Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: scylla-node-exporter.service: Start request repeated too quickly.
Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: scylla-node-exporter.service: Failed with result 'resources'.
Feb 24 00:43:25 ip-172-30-0-126 systemd[1]: Failed to start Prometheus exporter for machine metrics.
lines 1-9/9 (END)

Likely this is because running Scylla-4.4 too.
On the AMI built with master able to running scylla-node-exporter:
scyllaadm@ip-10-0-0-51:~$ systemctl status --no-pager scylla-node-exporter
● scylla-node-exporter.service - Prometheus exporter for machine metrics
     Loaded: loaded (/lib/systemd/system/scylla-node-exporter.service; enabled; vendor preset: enabled)
    Drop-In: /etc/systemd/system/scylla-node-exporter.service.d
             └─dependencies.conf, sysconfdir.conf
     Active: active (running) since Thu 2021-02-25 11:19:05 UTC; 1h 10min ago
   Main PID: 565 (node_exporter)
      Tasks: 5 (limit: 76216)
     Memory: 13.6M
     CGroup: /scylla.slice/scylla-helper.slice/scylla-node-exporter.service
             └─565 /opt/scylladb/node_exporter/node_exporter --collector.interr…

Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…one
Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…ime
Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…mex
Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…ues
Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…ame
Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…tat
Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…xfs
Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…zfs
Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…100
Feb 25 11:19:06 ip-10-0-0-51 node_exporter[565]: level=info ts=2021-02-25T11…lse
Hint: Some lines were ellipsized, use -l to show in full.

Switch build_deb_with_docker.sh container to Ubuntu 20.04.

@syuu1228 please test your fixes by running AMI artifacts test job: https://jenkins.scylladb.com/view/staging/job/scylla-staging/job/takuya/job/takuya-artifacts-ami-ubuntu-test/build?delay=0sec

Found timing bug.
Seems like Ubuntu 20.04LTS tries to check package update periodically, and it conflict with apt-get install systemd-coredump in scylla_coredump_setup.
I think we should avoid to run apt-get install on AMI instance first running time, it should be done in AMI creation time.
Will send fix both on scylla repo and this PR.
ar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6933]: E: Could not get lock /var/lib/dpkg/lock-frontend. It is held by process 2295 (unattended-upgr)
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6933]: E: Unable to acquire the dpkg frontend lock (/var/lib/dpkg/lock-frontend), is another process using it?
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6925]: Traceback (most recent call last):
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6925]:   File "/opt/scylladb/scripts/libexec/scylla_coredump_setup", line 49, in <module>
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6925]:     apt_install('systemd-coredump')
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6925]:   File "/opt/scylladb/scripts/scylla_util.py", line 709, in apt_install
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6925]:     return run(f'apt-get install -y {pkg}', shell=True, check=True, env=apt_env)
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6925]:   File "/opt/scylladb/python3/lib64/python3.9/subprocess.py", line 524, in run
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6925]:     raise CalledProcessError(retcode, process.args,
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[6925]: subprocess.CalledProcessError: Command 'apt-get install -y systemd-coredump' returned non-zero exit status 100.
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[484]: Traceback (most recent call last):
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[484]:   File "/opt/scylladb/scylla-machine-image/libexec/scylla_image_setup", line 31, in <module>
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[484]:     run('/opt/scylladb/scripts/scylla_coredump_setup --dump-to-raiddir', shell=True, check=True)
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[484]:   File "/opt/scylladb/python3/lib64/python3.9/subprocess.py", line 524, in run
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[484]:     raise CalledProcessError(retcode, process.args,
Mar 01 01:44:09 ip-10-0-0-79 scylla_image_setup[484]: subprocess.CalledProcessError: Command '/opt/scylladb/scripts/scylla_coredump_setup --dump-to-raiddir' returned non-zero exit status 1.

scylla_coredump_setup fix pushed, and sent new PR on https://github.com/scylladb/scylla/pull/8185

@bentsi AMI artifacts test succeed https://jenkins.scylladb.com/view/staging/job/scylla-staging/job/takuya/job/takuya-artifacts-ami-ubuntu-test/4/

@syuu1228 thanks. I am going to run short longevity (3h) using the AMI that you tested. If it passes, we can merge this PR
https://jenkins.scylladb.com/view/staging/job/scylla-staging/job/bentsi-staging/job/bentsi-longevity-3h/27/console

another thing, we need Releng team to be aware of this, I think the pipeline that builds SMI package should use dist/debian/build_deb.sh and the pipeline that builds the image aws/ami/build_deb_ami.sh.
@penberg can you assign somebody to do that?

short longevity finished. I didn't find any special issues. So from my side, we can merge the PR.
@syuu1228 are your changes breaking the current AMI build process?

short longevity finished. I didn't find any special issues. So from my side, we can merge the PR.
@syuu1228 are your changes breaking the current AMI build process?

We have scylla-machine-image-next that should be able to verify it

short longevity finished. I didn't find any special issues. So from my side, we can merge the PR.
@syuu1228 are your changes breaking the current AMI build process?

No, since all Ubuntu AMI related scripts are newly added, existing CentOS AMI is still remain and able to build.

I am merging this.
Since there is not answer from Pekka, I opened an issue in the scylla-pkg repo: https://github.com/scylladb/scylla-pkg/issues/1962


chore: code cleanup

We should merge this after we fixed the squash merge of master into beta, right?

That would probably be easiest

@gr2m Can you disable prettier-toc-me?

@gr2m Can you disable prettier-toc-me?

Done.


Test Bad Null foreign key

Explain your user case and expected results
Returns Host without any issues

Fixed by https://github.com/go-gorm/gorm/issues/3212


Reverse flag for time

Just an issue a bout the reverse flag.
Currently, the time loop is written at the first invocation of apply() with the current reverse property of the global time dimension. This is problematic if anither operator with a different flag has been created in the mean as it would change the time axis.

This is gonna disappear automagically once Navjot's arguments handling magic is in. So assigned to him

@FabioLuporini @navjotk Not necessarily, since we'll probably keep an explicit flag to indicate direction, as discussed in #247. The problem with this bug here is that the Iteration objects for time in both forward and backward operators read their direction from a global time dimension - and what's more they do this during .ccode, which is delayed until the first operator.apply(). So the first operator reads the dimension direction after the second one has changed it. The fix should be easy, and I'll try to come up with a test for the problem.

#283 offers a fix for this but maintains the stateful reverse flag on the global object representing the time dimension. This can potentially cause issues if two Operator constructors are running in parallel - one with reverse = True and one with reverse = False. I argue that having state on a global object is so evil, it should be avoided at all costs.
There have been various suggestions on how to avoid global state at the design level. I am compiling them along with the associated discussions here to enable further discussion:
Suggestion 1:

_schedule_expressions could pass the reverse flag to the Iteration constructor from the Operator's reverse property, without ever having to store it on the Dimension.

@mlange05 's response: schedule_expression creates (multiple) Iteration objects per Dimension, so somehow bypassing the parent object (dimension) when injecting additional information into the derivative object (iteration) defies the point of deriving it from the dimension in the first place. At the very least this sounds counter-intuitive, and no better than the current state hackery, at least IMHO.
My response: Deriving information from the parent object does not require storing any information on the parent. The same could easily be achieved by having a dict mapping Dimension objects to their direction (among many other possibilities). I don't see how creating multiple Iteration objects per Dimension makes anything different.
Suggestion 2:

Have a time_r dimension internally, which the Operator switches to, transparently, when it receives a time_axis=Backward

@mlange05 's response: Trickier than it might seem, since it effectively comes down to applying a subs({time: time_r}) over all received expressions. But what about t, or even some other derived t_s (eg. for sub-sampling)? What about time in the src/rec terms? I think this approach is really only safe if the user has full control of choosing the symbols for each term, but that makes the API slightly messier due to various built-in variants of "time".
Suggestion 3:

Allow Operators to accept dimensions at construction time (through kwargs), with the following syntax (definitely improvable):
Operator(..., t=REVERSE)
REVERSE is a special object (there can be many...) that is telling the Operator to traverse all iteration loops using dimension t in the reverse direction. The constructor propagates this information to schedule_expression, which will then know how to construct the t loops. This way, no internal state is changed. Also, the solution becomes completely general (we'd not be specialising time...)

@mlange05 's response: Hmmm ok, I don't really like this approach either, since it effectively encapsulates the functionality of generic "derived" dimensions but it forces us to encode each transformation in a special-case object, just like REVERSE. A more natural way would be to say time_r = DerivedDimension(expr=time_end - time) (or anything similar that permits generic expressions rather than hard-coding individual transformations) and then do Operator(... subs={time: time_r}). But at that point we are back to whether we should even use a derived dimension to encapsulate "reverse-time" or not and how to deal with granularity (see above discussion).
I would still argue for Suggestion 1 since it removes global state with no change in our API.

Excellent, thanks

As explained in the slack channel, this issue might now be a blocker for checkpointing.
Documenting another suggestion here (originally by @jhueckelheim ):
Every Operator make its own internal copy of the global dimension objects at construction time.

Every Operator make its own internal copy of the global dimension objects at construction time.

Technically not a solution, since it only decreases the critical region of code during which the potentially fatal interrupt in a multi-process settings could happen to "between entering the constructor and the dimension deep-copy".

I argue that having state on a global object is so evil, it should be avoided at all costs.

While true on a conceptual level, I in no way share your urgency, since I see this as an extremely unlikely corner case. Personal opinion on both sides though, I would say, so up in the air...

As explained in the slack channel, this issue might now be a blocker for checkpointing.

I have not fully understood your concerns here, so I will need to get back to you on this one.
On top of the comments above, I would argue that this issue is strongly linked to time-sub-sampling (issue #267) and other potentially related use cases where we derive one iteration space (dimension) from another. I would generally argue that whatever mechanism we come up with to solve this should also work for #267 and be generic, so that we can capture BCs on sub-regions or "expanding box" formulations as well. This of course is up for debate, so input welcome

Technically not a solution, since it only decreases the critical region of code during which the potentially fatal interrupt in a multi-process settings could happen to "between entering the constructor and the dimension deep-copy".

If you don't do anything to the global dimension before making the copy, the size of that region is 0.
I see no advantage to have Dimension objects globally shared between multiple Operators. Only disadvantages.

If you don't do anything to the global dimension before making the copy, the size of that region is 0.

That's still enough for an interrupt. Unless you make the combined copy-and-global-change operation atomic or use some lock on the variable you still haven't actually solved the synchronity issue. Btw, would you consider variable locking as a potential solution? @navjotk

It is my personal opinion that global state leads to race conditions? Should I even bother to give references for this one?

I see no advantage to have Dimension objects globally shared between multiple Operators. Only disadvantages.

Neither of these is under discussion here. In fact I do not like the global-state-change solution either (and was outvoted at the time), for one simple reason: the very existence of this issue demonstrates that on a conceptual level time != reverse_time (I am yet to hear a non-implementation related argument against this, btw). So, again, as stated before, I believe this is a self-inflicted problem, based on the fact that the conceptually clean solution (multiple objects/symbols; suggestion 2) becomes messy and inconvenient in practice due to having a large number of symbols.
Now, if we want to avoid the multi-symbol option by somehow forcing two conceptual dimensions onto the same symbol, I prefer the global state solution (my personal opinion), since the very low and contained risk from potential synchronicity issues for me is the lesser evils over manually forcing one iteration object to be special (by whatever means) during operator construction. My rational is that the hackery (justifiable by the "time is special" argument) should be contained to the highest level of abstraction we provide instead of deeply penetrating the iteration AST. Moreover, I think it should also be contained to the direction of time only, ie. things like sub-sampling, modulo buffers, sub-regions, etc should still have a generic "derivation mechanism" based on symbolic expressions rather than hard-coded operations.

That's still enough for an interrupt. Unless you make the combined copy-and-global-change operation atomic or use some lock on the variable you still haven't actually solved the synchronity issue. Btw, would you consider variable locking as a potential solution?

You don't need to change the global object at all. You make a local copy first, and only modify the local copy. These are atomic operations since global state does not change no matter where the Operator constructor is interrupted. The synchronicity issue exists because different Operators share the same Dimension objects. Removing that shared assumption, the issue being discussed here does not exist.

I'm not sure I understand how you want to do this then - aren't Dimensions globally cached SymPy symbols? How do you suppose to deep-copy that and then inject that copy into any received set of expressions?
In any case, I have outlined my reasoning above. If you are happy that this solves the race conditions issue, please go ahead.

Isn't his fix?


Add a table transform for acs show to match acs list.

Fixes https://github.com/Azure/azure-cli/issues/1611

@lostintangent

LGTM!

Proposed fields to display:
agent count, master count, orchestrator, public ssh key, resource group, location , dns-prefix, Master and agent FQDN's, agent vm size


[Migration Request]: 1.16.5 backport?

this is a fantastic mod, can you backport it to 1.16.5? i'm not familiar with 1.16.5 yarn, so i can't do it by myself

I am currently not planning to support versions older than 1.18, since it drastically increases the required maintenance. Also most of the modded playerbase is currently found in 1.19/1.20 and will shift to newer versions eventually.


Fix tilemap exporter

Need to fix sprite.png export and create a map that will allow the exporter to rebuild the sprite sheet exactly how it was when being loaded up

This was fixed.


Update README.md (a16b8a2) issue resolved

made some changes in the description

Hello @Khushi-Gupta13 !
Open an issue first and then link it here in the description.
Please follow the Contributing Guidelines you need to fill all the PR details which was mentioned.

#35 Enhancement of Readme.md file issue resolved.

Great job!
Thank you for contributing. Do give this repository a star if you feel like ✨


fix: pass the extra argument to event methods

Suggested edit:
diff --git a/src/dispatcher.h b/src/dispatcher.h
index 6bca0e3..96cb9f8 100644
--- a/src/dispatcher.h
+++ b/src/dispatcher.h
@@ -293,7 +293,7 @@ public:
      * @brief Parses and processes a JSON RPC request.
      *
      * @param request The JSON RPC request as a string.
-     * @param extra An optional JSON object that can be passed to the handler function (only for handlers added with add_ex()).
+     * @param extra Optional data that can be passed to the handler function (only for handlers added with add_ex()).
      * @return The response serialized into a JSON string.
      * @retval "" If the request is a [Notification](https://www.jsonrpc.org/specification#notification), the method returns an empty string.
      *
@@ -318,7 +318,7 @@ public:
      * @brief Processes a JSON RPC request.
      *
      * @param request The JSON RPC request as a `nlohmann::json` object.
-     * @param extra An optional JSON object that can be passed to the handler function (only for handlers added with add_ex()).
+     * @param extra Optional data that can be passed to the handler function (only for handlers added with add_ex()).
      * @return The response serialized into a JSON string.
      * @retval "" If the request is a [Notification](https://www.jsonrpc.org/specification#notification), the method returns an empty string.
      *
@@ -365,6 +365,10 @@ public:
      * @details This method does nothing by default. It is intended to be overridden in a derived class.
      * For example, it can be used to log requests or increment a counter.
      *
+     * @param extra Additional information that was passed to `process_request()`.
+     * Since `on_request()` is called before the request is parsed, the @a extra` parameter will not contain fields
+     * from the request itself (i.e., @a `extra['extra']` will not be set).
+     *
      * @note In the case of a valid [batch request](https://www.jsonrpc.org/specification#batch),
      * this method is invoked for every request in the batch but **not** for the batch itself.
      * However, if the batch request is invalid (e.g., is empty), this method is invoked once with an empty method name.
@@ -376,6 +380,9 @@ public:
      *
      * @details This method does nothing by default. It is intended to be overridden in a derived class. For example, it can start a timer to measure the method execution time.
      *
+     * @param extra Additional information that was passed to `process_request()`. If @a extra is a JSON object,
+     * it will contain all extra fields from the JSON RPC request (in @a extra['extra'], see `process_request()`).
+     *
      * @param method The name of the method to be called.
      */
     virtual void on_method(const std::string& method, const nlohmann::json& extra);
@@ -390,6 +397,9 @@ public:
      * @param code The result code: 0 if the method was processed successfully, or an error code
      * if an exception was thrown (e.g., exception::INTERNAL_ERROR)
      * or the request could not be processed (e.g., exception::INVALID_PARAMS).
+     * @param extra Additional information that was passed to `process_request()`. If the request had already been successfully parsed
+     * before `on_request_processed()` was called, the @a extra parameter will contain all extra fields from the JSON RPC request
+     * (in @a extra['extra'], see `process_request()`).
      */
     virtual void on_request_processed(const std::string& method, int code, const nlohmann::json& extra);

diff --git a/test/test_instrumentation.cpp b/test/test_instrumentation.cpp
index bd3262c..b10a324 100644
--- a/test/test_instrumentation.cpp
+++ b/test/test_instrumentation.cpp
@@ -34,9 +34,10 @@ TEST_F(InstrumentationTest, BadRequest)

     {
         const testing::InSequence s;
-        EXPECT_CALL(dispatcher, on_request(testing::_));
+        EXPECT_CALL(dispatcher, on_request(nlohmann::json::object()));
         EXPECT_CALL(
-            dispatcher, on_request_processed(std::string{}, wwa::json_rpc::exception::INVALID_REQUEST, testing::_)
+            dispatcher,
+            on_request_processed(std::string{}, wwa::json_rpc::exception::INVALID_REQUEST, nlohmann::json::object())
         );
     }

@@ -46,30 +47,43 @@ TEST_F(InstrumentationTest, BadRequest)
 TEST_F(InstrumentationTest, BatchRequest)
 {
     const auto input = R"([
-        {"jsonrpc": "2.0", "method": "add", "params": [1, 2], "id": 1},
-        {"jsonrpc": "2.0", "method": "subtract", "params": [2, 1], "id": 2},
-        {"jsonrpc": "2.0", "method": "add", "params": ["2", "3"], "id": 3},
-        {"jsonrpc": "2.0", "method": "bad", "id": 4}
+        {"jsonrpc": "2.0", "method": "add", "params": [1, 2], "id": 1, "extra": "extra1" },
+        {"jsonrpc": "2.0", "method": "subtract", "params": [2, 1], "id": 2, "extra": "extra2"},
+        {"jsonrpc": "2.0", "method": "add", "params": ["2", "3"], "id": 3, "extra": "extra3"},
+        {"jsonrpc": "2.0", "method": "bad", "id": 4, "extra": "extra4"},
+        {"extra": "extra5"}
     ])"_json;
     auto& dispatcher = this->dispatcher();

+    const auto extra_data       = R"({"ip": "<IP_ADDRESS>"})"_json;
+    const auto expected_data_1  = nlohmann::json({{"ip", "<IP_ADDRESS>"}, {"extra", {{"extra", "extra1"}}}});
+    const auto expected_data_2  = nlohmann::json({{"ip", "<IP_ADDRESS>"}, {"extra", {{"extra", "extra2"}}}});
+    const auto expected_data_3  = nlohmann::json({{"ip", "<IP_ADDRESS>"}, {"extra", {{"extra", "extra3"}}}});
+    const auto expected_data_4  = nlohmann::json({{"ip", "<IP_ADDRESS>"}, {"extra", {{"extra", "extra4"}}}});
+    const auto& expected_data_5 = extra_data;
+
     {
         const testing::InSequence s;
-        EXPECT_CALL(dispatcher, on_request(testing::_));
-        EXPECT_CALL(dispatcher, on_method("add", testing::_));
-        EXPECT_CALL(dispatcher, on_request_processed("add", 0, testing::_));
+        EXPECT_CALL(dispatcher, on_request(extra_data));
+        EXPECT_CALL(dispatcher, on_method("add", expected_data_1));
+        EXPECT_CALL(dispatcher, on_request_processed("add", 0, expected_data_1));

-        EXPECT_CALL(dispatcher, on_request(testing::_));
-        EXPECT_CALL(dispatcher, on_method("subtract", testing::_));
-        EXPECT_CALL(dispatcher, on_request_processed("subtract", 0, testing::_));
+        EXPECT_CALL(dispatcher, on_request(extra_data));
+        EXPECT_CALL(dispatcher, on_method("subtract", expected_data_2));
+        EXPECT_CALL(dispatcher, on_request_processed("subtract", 0, expected_data_2));

-        EXPECT_CALL(dispatcher, on_request(testing::_));
-        EXPECT_CALL(dispatcher, on_method("add", testing::_));
-        EXPECT_CALL(dispatcher, on_request_processed("add", wwa::json_rpc::exception::INVALID_PARAMS, testing::_));
+        EXPECT_CALL(dispatcher, on_request(extra_data));
+        EXPECT_CALL(dispatcher, on_method("add", expected_data_3));
+        EXPECT_CALL(dispatcher, on_request_processed("add", wwa::json_rpc::exception::INVALID_PARAMS, expected_data_3));

-        EXPECT_CALL(dispatcher, on_request(testing::_));
-        EXPECT_CALL(dispatcher, on_request_processed("bad", wwa::json_rpc::exception::METHOD_NOT_FOUND, testing::_));
+        EXPECT_CALL(dispatcher, on_request(extra_data));
+        EXPECT_CALL(
+            dispatcher, on_request_processed("bad", wwa::json_rpc::exception::METHOD_NOT_FOUND, expected_data_4)
+        );
+
+        EXPECT_CALL(dispatcher, on_request(extra_data));
+        EXPECT_CALL(dispatcher, on_request_processed("", wwa::json_rpc::exception::INVALID_REQUEST, expected_data_5));
     }

-    dispatcher.process_request(input);
+    dispatcher.process_request(input, extra_data);
 }


ExternalIPs/LoadBalancers services do not honor externalTrafficPolicy

The docs have a general paragraph on externalTrafficPolicy that explain Cluster vs Local settings. They do not explain this setting is (seems?) only honored by ClusterIP services (or: the cluster IPs in general), but not by ExternalIPs/Load Balancers. The docs seem to be general on both kinds.
#3408 indicates this is only supported by Kubernetes 1.18+ (and if a feature toggle is set) and missing in Calico entirely yet.
Expected Behavior
Feature implemented or docs unambiguous.
Current Behavior
Features seems not implemented, but docs are not explicit about this.
Possible Solution
Implement feature or make docs explicit.
Steps to Reproduce (for bugs)

Context

Your Environment

Calico version 3.14.0
Orchestrator version (e.g. kubernetes, mesos, rkt): Kubernetes 1.14.1
Operating System and version: Ubuntu 18.04
Link to your project (optional):

Looks like I was misleaded by the fact that Calico only honours the externalIPs field, not loadBalancerIP (and further misconfiguration/test configuration in BGPConfigurations: serviceExternalIPs vs serviceClusterIPs). Got it working now. #2770 finally explained the situation.

After understanding what was wrong, Kubernetes and Calico behavior seems correct to me (but was not obvious on first hand). Would you mind a minor pull request indicating that Calico only exports the externalIPs fields for https://docs.projectcalico.org/networking/advertise-service-ips#advertising-service-ips-quick-glance? If you agree on having some note, I'll gladly provide a pull request.

Sorry for the delay - I think a docs PR here is fine, although that doc already seems pretty clear to me.
Advertising LB IPs is a further enhancement we could make to the code as well.


Start stop issues

Looking into problems with sox play and mpg123, I've found an issue caused by the way these applications close and then immediately re-open a bluealsa device, and also with mpg123 in particular that an application can cause the client to call bluealsa_stop() twice.
This PR attempts to fix these issues. See comments against each of the two commits for further details.

@borine I've made the modification that I've mention in the commend above and pushed amended commit to master. I hope you don't mind. Many thanks for spotting this bug (and resolving issues with mpg123)!
I've pushed only the commit with IO thread synchronization, though. I think that the other one is not necessary. If I'm wrong, please provide some example which fails without this "no-op stop" commit.

Thanks, I can confirm your commit fixes the issues that this PR is intended to address. Sox play now works correctly with all my test files on both Intel and raspberry pi.
mpg123 is not yet fully fixed, but that's a different story and I'll raise an new issue to discuss that.
Closing this PR as complete.


Update how-to-use-a-transactional-template-with-smtp.md

This is to organize x-smtpapi content under its own group. This goes with https://github.com/sendgrid/docs/pull/4248

Hello @kylearoberts,Thanks again for the PR! It's HACKTOBERFEST! We want to show our appreciation by sending                                              you some special Hacktoberfest swag. If you have not already, could you please fill out                                              this form so we can send it to you? Thanks!                                             Team SendGrid DX


refactor(storage): remove unnecessary ref to local_version_manager

What's changed and what's your intention?
Lift LocalVersionManager::pick_few_tables to SstableStore::sstables, then in turn removing some unnecessary references in LocalVersionManager, CompactorContext and SharedBufferUploader.
Checklist

[ ] I have written necessary docs and comments
[ ] I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.


Sign in and sign out page enhancement

Title of the Pull Request - Sign in and sign out page enhancement

[x] Have you renamed the PR Title in a meaningful way?

Related Issue

Closes: #1093
Description

Changed the style of the Sign in and sign out page
How Has This Been Tested? ⚙

When there is a issue , i open the inspect option in web browser and see where the problem is happening.
Screenshots 📷

Type of change
What sort of changes have you made:

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Code style update (formatting, local variables)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

Checklist

[x] My code follows the code style of this project.
[x] I have followed the contribution guidelines
[x] I have performed a self-review of my own code.
[x] I have ensured my changes don't generate any new warnings or errors.
[x] I have updated the documentation (if necessary).
[x] I have resolved all merge conflicts.

Can you add level 3 as it takes lots of time to make it. @vishanurag

@souvikpramanikgit You just copied this entire page from a different project and pasted it here.