Replaying subscribing event processors

Hi,
I outlined my idea basically here already. I would like to provide an actual PR for this feature, but want to get some input first. First and foremost: Is this feature interesting for Axon?
Feature Description
For technical reasons we are forced in our application to use some subscribing event processors. This means that it is rather difficult to migrate the corresponding read models and we would like to simply reset the subscribing event processors as it is possible with tracking event processors.
Here is my suggestion for this feature (I omitted the less interesting reset-methods) within the SubscribingEventProcessor.
public <R> void resetTokens(TrackingToken startPosition, R resetContext) {
    Assert.state(supportsReset(), () -> "The handlers assigned to this Processor do not support a reset");
    Assert.state(!isRunning(), () -> "Processor must be shut down before triggering a reset");
    StreamableMessageSource<TrackedEventMessage<?>> streamableMessageSource = getMessageSourceAsStreamableMessageSource( );

    transactionManager.executeInTransaction(() -> {
        eventHandlerInvoker().performReset(resetContext);
    });

    BlockingStream<TrackedEventMessage<?>> stream = streamableMessageSource.openStream(startPosition);
	while (stream.hasNextAvailable( )) {
		transactionManager.executeInTransaction( ( ) -> {
			try {
				TrackedEventMessage<?> eventMessage = stream.nextAvailable( );
				if ( eventHandlerInvoker().canHandle( eventMessage, Segment.ROOT_SEGMENT ) ) {
					eventHandlerInvoker().handle( eventMessage, Segment.ROOT_SEGMENT );
				}
			} catch ( final Exception ex ) {
				throw new IllegalStateException("The reset of the event processor failed. The processor might be in an undefined state.");
			}
		} );
	}
}

public boolean supportsReset() {
    return eventHandlerInvoker().supportsReset() && messageSource instanceof StreamableMessageSource;
}

We would have to add a warning to this method of course, as events published between shutdown and start would be lost.
Current Behaviour
There is no API to reset subscribing event processors.
Wanted Behaviour
There is an API to reset subscribing event processors, similar to tracking event processors.

First and foremost, thanks for making an issue with us again, @nils-christian.
We as a team value the input you and your colleagues put into Axon Framework from a user's perspective.
Now, to the request at hand.
Conceptually speaking, I would not anticipate a SubscribingEventProcessor to be able to "reset its position on the stream," because it does not deal with a message stream, to begin with.
The intent of this processor is to subscribe itself to a source, receiving the events as they're published.
From that concept, I don't see how a subscribing processor can move back within the set of received events.
Perhaps something like a "local set of handled events" could be maintained, likely with a retention period, which it could reset through.
But, in all honesty, that also sounds like duplicating the event stream (which might be the event store) of that one application, in essence building a "streaming message source."
And with a streaming message source, I'd expect the usage of a StreamingEventProcessor, like the TrackingEventProcessor or PooledStreamingEventProcessor.
Now, I don't want to eliminate your idea entirely just yet, though.
Because your request does come from a production environment angle, I assume.
With that said, I'd like to focus on one specific line in your issue:

For technical reasons we are forced in our application to use some subscribing event processors.

What are these technical reasons?
And a little futher on, why does this merit adjusting the conceptual idea of the SubscribingEventProcessor?

Hi @smcvb,
We have various points in our code which use subscribing event processors for various reasons.
One of them is that we use a process engine and have to "marry" both Axon and said process engine (the reasons for using the process engine are a little bit more difficult).
The process engine is able to put on a veto on a command (more precisely: on the event published from the aggregate) based on its state. This wouldn't be possible if the handling of the events to control the process engine would be done asynchronously in a tracking event processor. If, on the other hand, the aggregate would control the process engine in the commands, we would clutter the aggregate with a lot of process engine specific code (trust me, we have been there).
Now, the process engine is, thanks to a task listener, still responsible for publishing events for a central task list. That means that it needs a read model (filled by the events from the aggregate) to publish those events. If the read model projector (the event handler) wouldn't be subscribing, we couldn't make sure that the read model is correctly filled when the task listener publishes its events.
So what happens is the following:

Command is sent to the aggregate
Event is published from the aggregate
A first subscribing EventHandler processes the event and fills a read model for the task listener.
A second subscribing EventHandler processes the event, checks if the process is in the correct state and controls the process engine accordingly.
The task listener is informed and uses the now filled read model to publish a new event for the task list.

All of this happens synchronously and in the same transaction. I am aware that this is far from optimal, but so far we do not see another (working) solution. I am aware that we could at least move the control of the process engine into a tracking event processor, but this means that the state of the aggregate and the state of the process could deviate.
That means, we have multiple subscribing event handler which also require a read model. Resetting the event handler would allow us to simply rebuild the read model without triggering the process engine again.
Another point in our code is the usage of read models for the UI. We sometimes send a command (synchronously) and expect that the UI model has been updated when the command returns to the rest controller so that the rest controller can return parts of the modified UI model to the caller.
So what happens is the following:

Command is sent to the aggregate
Event is published from the aggregate
A subscribing EventHandler processes the event and fills a read model for the UI.
The rest controller reads the modified UI model and returns it to the caller.

Best regards
Nils

Thanks for sharing your scenarios with us, @nils-christian.
I got two points on your reply, and an ending note:

One of them is that we use a process engine and have to "marry" both Axon and said process engine (the reasons for using the process engine are a little bit more difficult).
In order to make sure that the state of the aggregate and the process engine are in sync, the event handler controlling the process engine has to be subscribing.
Also task listeners, fired from the process, require an own read model.

Based on this description, I don't immediately see why the thread handling the command should also be responsible for kicking of this process engine.
If the process engine "event handler" just handles the events once it's there, it'll kick of the process from that point on.
Unless you've made a promise to your business and user that these needs to be synchronous, I think you could use a Streaming Event Processor here.

Another point in our code is the usage of read models for the UI.

Although I know it's easier said than done, it would be optimal if the UI would deal with the fact it works with an eventually consistent system.
As such, it would be more flexible if it would subscribe to specific query models it requires, so that once these are updated (through a command), they'll be there once they're there.
Or, you could spoof synchronous behavior, by performing a subscription query right before dispatching a command.

As you might read from the above, I think that (with my limited knowledge of your application) there are perhaps some ways to mitigate the usage of the Subscribing Event Processor.
Regardless of that though, I still stick to my original point of view that the Subscribing Event Processor and Subscribable Message Source conceptually do not support the behavior of replaying events from a point in time.
If you really need to stick to the SubscribableMessageSource, you could construct some custom logic to "republish" the events from some point in time on the message source.

With that said, I will close this issue for the time being.
Perhaps future insights will change this issue to be opened again.
And, definitely feel free to keep replying in this thread!

Hi @smcvb,
I understand.
Just to provide a backlink for future readers: A potential implementation for this can still be found in the discussion forum.


Add passthrough for RefreshControl

It's nice to be able to use React Native's RefreshControl to show a loading indicator and pull to refresh inside of a tab group.
This PR adds a passthrough so you can use it as you would with a normal ScrollView

You can pass refreshControl with contentProps


feat(weave): Optionally disable code logging

Summary
Resolves
https://wandb.atlassian.net/browse/WB-20322

Closed in favour of https://github.com/wandb/weave/pull/2428


Rename extension on GitHub

Hi Simon,
This issue is being opened to let you know that your extension will need to be renamed here on GitHub via the 'Settings' option to reference Camunda Platform 7 rather than Camunda BPM. if you have any questions or require any assistance, please do not hesitate to reach out to @camunda-community-hub/devrel! :) You can also make use of Shields.io badges to indicate which version of Camunda your extension is compatible with.

Hi @zambrovski is this something you can take care of or are interested in doing? If renaming the entire project is too difficult, I would at least recommend renaming Camunda BPM to Camunda Platform 7 in the README and adding the badge.

Will do.. please let me wait until after the CCS2022

Of course 😄

By the way, somehow, I'm missing most permissions in "settings" could you please check and provide them back to me?

I went ahead and gave you admin level for this repo.

Do we need to change Maven Coordinates too? Currently they are
<groupId>org.camunda.bpm.extension</groupId>
<artifactId>camunda-bpm-process-test-coverage-junit4</artifactId>

I believe those are still the same, but I'm tagging in @berndruecker to help confirm 😄

I'll change them too.


Not compatible with django 3.2.*

This appears to be related to the app discover change, but I was not able to confirm.
https://docs.djangoproject.com/en/3.2/releases/3.2/

#21


changes for flagging noisy pixel

This PR contains the proposed changes on flagging noisy pixels.
I would not merge this until Kevin has time to look at it and agree or disagree that this is what we want.

@bhilbert4 I am at STScI this week. We can talk about this change and get it merged if you think
it is correct thing to do. Basically the stats for flagging noisy pixels are determined from the clean stack of slope images (removing those slopes with cosmic rays) but the noisy cut is done on the
original array.

Hmm. But doesn't this risk flagging some pixels as noisy simply because cosmic rays have artificially increased the stdev of their slopes?


DeprecationWarning: grpc.load

Getting error "DeprecationWarning: grpc.load: Use the @grpc/proto-loader module with grpc.loadPackageDefinition instead" when starting the node application.

This should be fixed now.


ignore .idea

Description of changes
I'm not using VSCode but Jetbrains tools. Added ".idea" to gitignore
Describe changes here

Usually .ignore files are alphabetically ordered as convention for ease of finding entries


keep_prob always 1.0?

Hello,
Why is keep_prob always 1.0?
Does that mean that the dropout in the discriminator is always disabled?
Thank you.

Hi Ramiro,
Yes, I initially played around with the dropout value a bit.
But eventually I'd decided not to use dropout at all since the model worked fine without it.
(I should have deleted the parameter "dropout:1.0", but I thought I might want to use dropout again in the future)
But you are welcome to try tuning the dropout value for extra performance boost.
Thanks,
Ed


Timer STM32 #error This code is designed to run on STM32F/L/H/G/WB/MP1 platform!  Please check your Tools->Board setting.

I have installed the library of TimerInterrupt_Generic for ArduinoSTM32 module via Arduino IDE, so i have tried to test the example of library as you can see below:

and i have gotten this error:

TimerInterruptLEDDemo:45:4: error: #error This code is designed to run on STM32F/L/H/G/WB/MP1 platform! Please check your Tools->Board setting.
   #error This code is designed to run on STM32F/L/H/G/WB/MP1 platform! Please check your Tools->Board setting.
    ^
In file included from /home/so/snap/arduino/current/Arduino/libraries/TimerInterrupt_Generic/examples/STM32/TimerInterruptLEDDemo/TimerInterruptLEDDemo.ino:64:0:
/home/so/snap/arduino/current/Arduino/libraries/TimerInterrupt_Generic/src/TimerInterrupt_Generic.h:86:4: error: #error Unsupported Board! Please check your Tools->Board setting.
   #error Unsupported Board! Please check your Tools->Board setting.
    ^
exit status 1
#error This code is designed to run on STM32F/L/H/G/WB/MP1 platform! Please check your Tools->Board setting.

Also, This question is asked here:
https://arduino.stackexchange.com/questions/79638/timer-stm32-error-this-code-is-designed-to-run-on-stm32f-l-h-g-wb-mp1-platform

Check STM32_TimerInterrupt Library Error

The Roger Clark's STM32 core  you're using is not currently supported. I don't know in the future if I can add that core into the supported list.
You have to use this Arduino_Core_STM32.
Convert to new core by following the intructions in Getting Started


validation for phone number

It currently only allows only 10 characters, But any character is allowed. We could restrict that to only numbers.

I'm working on it.

I'm almost done with this, pls do not duplicate

Ok @joicemjoseph

This can be closed as #84 subsumes it.

Duplicate #84


Allow filtering of metric calculations when flushing

Rather than always reporting all calculated values on metric types such as percentiles, mean etc, allow these to be excluded when reported to save storage in the underlying TSDB, TSDB query features can then be used to perform equivalent calculations when applications don't need them.

Support for all fields in 2.1.0 except counter and meter values due to breaking change required in IMetricSnapshotWriter.
Related: https://github.com/AppMetrics/AppMetrics/issues/270


Windows - hardware back button problem

Ionic version:  (check one with "x")
[ ] 1.x
[X ] 2.x
I'm submitting a ...  (check one with "x")
[ X] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/
Current behavior:
Windows - hardware back button stops working after clicking it once while on root pages (exists/suspends)
Expected behavior:
back button should always work
Steps to reproduce:
starter tab project (ionic start --v2 myApp tabs).

build for windows and deploy on a windows device.
then click back button on one of the root pages (app goes to background).
open the app from background
click back button again

after some debugging, I think the code that generates the problem is in Platform implementation:
var Platform = (function () {
    function Platform() {
..
..
        this.backButton.subscribe(function () {
            // the hardware back button event has been fired
            (void 0) /* console.debug */;
            // decide which backbutton action should run
            _this.runBackButtonAction();
        });
    }

on Windows, an exception is thrown when trying to exit the app, as Cordova specifies (http://cordova.apache.org/docs/en/6.x/cordova/events/events.html#windows-quirks)
the exception unsubscribes the handler, so the handler isn't called anymore on back button events.

@almost92 could try try this fix?
https://github.com/driftyco/ionic/pull/11017

@manucorporat tested, it doesn't suspend the app anymore.
(clicking hardware back button from root page doesn't send the app to the background)

@manucorporat tested, it doesn't suspend the app anymore.
(clicking hardware back button from root page doesn't send the app to the background)

With the fix I get this error '0x800a138f - JavaScript runtime error: Unable to get property 'exitApp' of undefined or null reference' when debugging with Visual Studio. This crashes the app...

@digaus is cordova running?

I am building it as a windows application and then run it.


Action box of Images isn't available for role "User"

Action box on image list and image sidebar isn't available for User with role "User"
Actual result: Action box of Images isn't available for role "User"
Expected result: Action box of Images is available for role "User"
Connected feature: image_actionbox

Test on:
tamazlykar/1006-action-box-on-image-list
Regression:
image_actionbox

Action box in Image sidebar isn't available for role "User" (Template and ISO)
Steps:

Log in as role User and go to Images
Press to any User's Image
Go to "Template" tab in Image sidebar

Actual result: Action box is missing at top of Image sidebar.
Expected result: Action box is present at top of Image sidebar.
Image Sidebar of User

Test on:
tamazlykar/1006-action-box-on-image-list
Regression:
image_actionbox

@tamazlykar
Tested on tamazlykar/1006-action-box-on-image-list in all browsers (except IE - blocked)


cert_manager: retry creating test certificate for 90s

Fixes kubernetes-sigs/cluster-api#4559
In using the cert_manager extension, the final test certificate step often fails with this error:
Beginning Tiltfile execution
Installing cert-manager
Waiting for cert-manager to start
Testing cert-manager
Traceback (most recent call last):
  /Users/matt/Projects/src/sigs.k8s.io/cluster-api-provider-azure/Tiltfile:339:24: in <toplevel>
  /Users/matt/Projects/src/sigs.k8s.io/cluster-api-provider-azure/tilt_modules/cert_manager/Tiltfile:61:10: in deploy_cert_manager
  <builtin>: in local
Error: command ["sh" "-c" "cat << EOF | kubectl apply -f - \napiVersion: v1\nkind: Namespace\nmetadata:\n  name: cert-manager-test\n---\napiVersion: cert-manager.io/v1\nkind: Issuer\nmetadata:\n  name: test-selfsigned\n  namespace: cert-manager-test\nspec:\n  selfSigned: {}\n---\napiVersion: cert-manager.io/v1\nkind: Certificate\nmetadata:\n  name: selfsigned-cert\n  namespace: cert-manager-test\nspec:\n  dnsNames:\n    - example.com\n  secretName: selfsigned-cert-tls\n  issuerRef:\n    name: test-selfsigned\nEOF"] failed.
error: exit status 1
stdout: "namespace/cert-manager-test created\n"
stderr: "Error from server (InternalError): error when creating \"STDIN\": Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": dial tcp <IP_ADDRESS>:443: connect: connection refused\nError from server (InternalError): error when creating \"STDIN\": Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": dial tcp <IP_ADDRESS>:443: connect: connection refused\n"

I investigated and found even though the deployments are Available and the relevant pods are Running, the webhook still won't respond for some amount of time on first startup. Possibly this is what cert-manager installation docs refer to when they say:

It may take a minute or so for the TLS assets required for the webhook to function to be provisioned. This may cause the webhook to take a while longer to start for the first time than other pods.

Since the Starlark language doesn't offer exception handling, and kubectl itself has no "retry on fail" capability, I wrote a simple shell loop to try creating the test certificate every fifteen seconds for a minute and a half before giving up. In my testing, this gets our make tilt-up working every time, whereas before it failed consistently.
I also updated the cert-manager default version to the current v1.3.1 and fixed some language in the README.

don't spend time investigating the CI failure, circleci is having a major outage right now

cc: @fabriziopandini @davideimola @CecileRobertMichon @jackfrancis @devigned

lgtm

I did not had time to investigate it properly but I was playing around the idea of

It may take a minute or so for the TLS assets required for the webhook to function to be provisioned. This may cause the webhook to take a while longer to start for the first time than other pods.

It seems that cert-manger injects a secret on its own web hooks
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  annotations:
    cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca

So I'm I'm wondering if it would be cleaner to wait for this certificate to exists instead of retrying apply of our test yaml.
(wait for kubectl get secrets cert-manager-webhook-ca -n cert-manager to return something)

to speak to @fabriziopandini 's question - i definitely think there are better ways to do the wait here to make this faster / more responsive, but in the meantime this solution doesn't hurt. i'm going to add some tests for this and play around a bit.

I definitely think there are better ways to do the wait here

I agree it would be cleaner to do kubectl get secrets cert-manager-webhook-ca -n cert-manager than it is to retry creating the certificate. I didn't dig as deep as @fabriziopandini because I thought waiting for anything underneath the deployment is kind of implementation-specific and might break in the future.
But it seems unlikely they would change the cert-manager secret, so if that approach works, let's do replace the retry loop with that. It would also be more consistent and easier to read.

I tried to create a follow-up PR with the above change we talked about, but since it's a secret, I don't think we can use kubectl wait with it. It doesn't have a Status field. and kubectl wait doesn't yet have a syntax to specify waiting for a resource to exist.
Additionally, there are multiple secrets that I assume cert-manager needs before it's fully functional. Usually I have seen that the canonical cert-manager-webhook-ca is the first secret to appear, and the others with generated names show up several seconds later. This might mean that waiting for that one secret is insufficient.
% k get secret -n cert-manager
NAME                                  TYPE                                  DATA   AGE
cert-manager-cainjector-token-lhpds   kubernetes.io/service-account-token   3      9m43s
cert-manager-token-cmvwl              kubernetes.io/service-account-token   3      9m43s
cert-manager-webhook-ca               Opaque                                3      9m33s
cert-manager-webhook-token-jm4hm      kubernetes.io/service-account-token   3      9m43s
default-token-n5dp9                   kubernetes.io/service-account-token   3      9m43s


windows build ok but run failed with TI_ERROR_ARGUMENT_NULL

Hi maintainers,
This time I build it on windows. Still use same customized configs:

turn off TI_WITH_LLVM
turn on TI_WITH_VULKAN
turn on TI_WITH_C_API

it build ok for both taichi and taichi-aot-demos with Visual Studio 2022, but running with error TI_ERROR_ARGUMENT_NULL, as shown in the screenshot:

0. Machine Environment

Visual Studio 2022 17.3.5
cmake version 3.24.3
vulkan SDK <IP_ADDRESS>
vulkaninfo result: vkinfo.txt
GPU device: Intel Iris Xe Graphics

1. how I build taichi
taichi repo with latest commit
* 7a5dac064 2022-11-08 | [lang] MatrixType bug fix: Allow builtin any() and all() (#6533) (HEAD -> master, origin/master, origin/HEAD) [Yi Xu]

build script is d:/github/taichi/build.cmd with contents:
@echo off

set TAICHI_CMAKE_ARGS=-DCLANG_EXECUTABLE=D:/soft/taichi-clang-10.0.0/bin/clang++.exe -DLLVM_AS_EXECUTABLE="D:/soft/taichi-llvm-10.0.0-msvc2022/bin/llvm-as.exe" -DTI_WITH_VULKAN=ON -DTI_WITH_C_API=ON -DTI_WITH_LLVM=OFF

python setup.py install > log.txt 2>&1

The build log is:
taichi-build-log-windows-vs2022.txt
how I build taichi-aot-demo
taichi-aot-demo with latest commit
* 9994029 2022-11-08 | Fixed renderer initialization (#53) (HEAD -> master, origin/master, origin/HEAD) [PENGUINLIONG]

build script is d:/github/taichi-aot-demo/build.cmd with contents:
@echo off

set TAICHI_C_API_INSTALL_DIR=D:/github/taichi/_skbuild/win-amd64-3.9/cmake-install/c_api
cmake -S . -B build > configure-log.txt

The cmake configure log file is:
taichi-aot-demo-configure-log.txt

I cannot figure out what is wrong from the call stack: (E2_mpp88_headless):


Account Linking Phase 1

See #1179 and #309
TODO:

[x] Handle register email flow
[x] Handle the 'welcome back' email flow
[x] Handle IDP flows
[x] Handle phone flow
[ ] Update docs
[ ] Add comprehensive tests

@SUPERCILEX am I correct interpreting #309 to allow linking between any pre-signed-in account and not just anonymous?

I am also a little confused how to handle email.
Let's say after CheckEmailFragment we know that the email already exists.  Then we know that calling linkWithCredential will fail, so we are definitely going to have a merge conflict.
But, we want to make sure that in the merge conflict handler the AuthCredential object they get is always going to be valid ... and the only way to check if the email credential is valid is to attempt a sign-in which will destroy the Anonymous auth state.
Seems like in #309 you'd give the user a chance to read all data from the anon account before attempting the link, but not sure how it would work here.

Removing this from 3.3.0 as #1189 dropped a nuclear merge bomb on this :-)

Oh noes! 😞 I completely forgot about this PR, sorry man!

@SUPERCILEX haha no worries, I knew exactly what I was getting into.

@SUPERCILEX I've got the merge conflicts squared away.
The two places I am not sure how to apply the linking logic are in SocialProviderResponseHandler and LinkingSocialProviderResponseHandler.
Mind taking a look?

Will do! I just realized I forgot some loading stuff in the providers... so PR incoming for that first! 😊

Testing:
https://github.com/SUPERCILEX/FirebaseUI-Android/blob/460f4307089d3933e2517058758147d725eea567/auth/src/main/java/com/firebase/ui/auth/ui/idp/CredentialSignInHandler.java#L105-L127

@SUPERCILEX thanks for the comments.
The temporary FirebaseApp trick isn't to check for a collision, it's for when you know there's going to be a collision and want to make sure you're giving the user a valid AuthCredential.  So you can check if the credential is valid but without messing up the current sign-in state, so they still have a chance to transfer data.
As for why we're not doing the Service thing: basically because in the API discussion for doing this feature cross-platform the only thing we could agree on in the short term was something minimal and correct like this, while leaving room to improve the developer experience in the future.

Closing this PR, it's very very conflicty and @lsirac is going to take over this task.


Adding a new section to the Markdown course

A new addition to the Markdown course.
Topic
Best practices and tips in markdown.
Sub-topics

Consistent formatting and styles in markdown.
Tools for markdown editing.

You have added that already


implement itoa for int64 handling + unit-tests

Could you check this? @yogpan01 @anttiylitokola @kuggenhoffen
I changed most snprintf functions, only leaving some which handled only small ints.
I defined BUFFER_SIZE seperately for most files, also increasing it for large buffers 20->21. The 20 was a bit too small for maximum int64 values.
New unit-tests also included in this commit.

The test O5030400F002 passes when run locally.

rebuild


Paste only values from Excel

Hi,
Can I paste only the values from Excel on Jexcel cells. For example if I have number formated 1.062.606,09, paste the value 1062606,09 in Jexcel?
Thanks for the great work with this project.

You can use onbeforepaste event to parse anything add to the table.


DGraph UI enhancement ideas

A couple suggestions for the DGraph UI to make it more useable:

The ability to set JSON as the default viewing option. For example if you select JSON perhaps all further queries can remain on JSON, if you toggle back to Graph then all further queries (or eventually expanded queries) display the same mode.
Ability to select the entire response body. We find ourselves copy and pasting between that screen very often. So a click to select all or click to select and copy to clipboard would be wonderful
Having the query re-iterated on the left side of the expanded query window seems a bit useless. Suggestions: allow the left panel to be your query editor panel and your response on the right side or put the graph on the left and JSON on the right. Would be great to select the layout you'd prefer.
Saving queries or pinning them would be helpful.
Place the bottom results bar listing the nodes and label legend above the query/response panel so it's easier to find instead of having to scroll to it as a quick reference. Matter of fact perhaps the left hand panel could be your legend (vertically stacked) and the right hand can be your response window.
If you have more than 5 queries perhaps they can be hidden under a "More" option.
Move close all to the left hand navigation bar so it's accessible anymore in the UI.

That's all I've got for now =)

1, 2, 4, 6, 7 fixed or deprecated.
3,5 are probably no longer relevant with layout completely redesigned


PR: Split Variable Explorer module

Move variable explorer to a module
variableexplorer/
├── confpage.py
├── __init__.py
├── plugin.py
├── tests
│   ├── __init__.py
│   ├── issue_2514.csv
│   ├── issue_3896.csv
│   ├── test_arrayeditor.py
│   ├── test_collectioneditor.py
│   ├── test_dataframeeditor.py
│   ├── test_importwizard.py
│   ├── test_namespacebrowser.py
│   ├── test_texteditor.py
│   ├── test_utils.py
│   └── test_variableexplorer.py
├── utils.py
└── widgets
    ├── importwizard.py
    ├── __init__.py
    ├── namespacebrowser.py
    └── viewers
        ├── array.py
        ├── collections.py
        ├── dataframe.py
        ├── __init__.py
        ├── object.py
        └── text.py

~This have to be rebased after #4548~ done

I'd like to move our viewers to spyder/plugins/variableexplorer/viewers to have a smaller import path.
Besides, our viewers are not really plugin widgets, i.e. they are not embedded in the plugin but can work independently of it. So that's another reason to have them out of widgets.
@goanpeca, what do you say?

Are these widgets core so that they go inside spyder/widgets/ or they really only make sense for the variable explorer?
@ccordoba12 I dont think having a shorter import path should be a reason to break a nicely structured plugin. Viewers are widgets so they should live inside the widgets module, we just happen to group them inside a submodule called viewers cause is nicer. So I am :-1: on that.
We should be using relative imports anyway

Ok, no problem. Then @rlaverde, please make all import statements here to fit in 80 columns, e.g.
from spyder.plugins.variableexplorer.widgets.viewers.collections import (
CollectionsEditor)
etc.
I really don't like to have longer lines :-)

Ok, no problem. Then @rlaverde, please make all import statements here to fit in 80 columns

It's that really necessary? unnecessary wrapped lines doesn't look good, sometimes is better a line longer than 80 characters. (yapf philosophy, i.e. if I yapfy this files the long imports didn't change.)
We could also consider changing the folder name "variableexplorer" is 16 characters long, "namespace" or "workspace" will be only 9.

We could also consider changing the folder name "variableexplorer" is 16 characters long, "namespace" or "workspace" will be only 9.

I would support that :+1:

It's that really necessary? unnecessary wrapped lines doesn't look good, sometimes is better a line longer than 80 characters.

I think modern software development no longer requires this outdated rule.  I think using 100-ish is quite normal these days, so keeping on 79 for the sake of an old standard is not relevant nowadays.

We could also consider changing the folder name "variableexplorer" is 16 characters long, "namespace" or "workspace" will be only 9.

Not for this PR :-) If/when we rename this plugin's name, then we'll change its namespace name too.

I think modern software development no longer requires this outdated rule. I think using 100-ish is quite normal these days, so keeping on 79 for the sake of an old standard is not relevant nowadays.

All our code adheres to the 79 limit and this is just a refactoring PR, so this is not the place to start changing it.
So @rlaverde, please fix those imports and also all lines that are wider than 79 columns (except for test files).

@rlaverde is this ready?

@rlaverde is this ready?
It is missing the fixes in lines wider than 79, I think that I should manually change them (no magic regex and sed 😢 )

I think that I should manually change them

Sorry for that, but I think there are no more than 10 of those ;-)

@rlaverde please fix that so we can keep moving with the split !

To solve the errors in our tests, please apply here the changes I did in PR #4613.

To solve the errors in our tests, please apply here the changes I did in PR #4613.

Done, I cherry-picked the commits

Tests for PyQt4 are faliing, I don't know why :-/

I think it's related to the error solved in 70c2b95390648deef831ccb27a9c4eabaf4e87e7 (cherry-picking 🙈 )

You were right, now these tests are passing ;-) Merging, since this is ready.
I just have an unrelated question: what will happen with those cherry-picked commits when we merge with master?

I just have an unrelated question: what will happen with those cherry-picked commits when we merge with master?

I bet (and hope) git is smart enough to notice that both commits introduced the same changes and won't be merge conflicts.

@rlaverde bet or hope :-p you can't have both ;-)


[v3 beta] Android bugs

Thank you for your amazing work. It's so minimal looking great ntp extension.
As it's not made for mobile phones there're some ui bugs in beta version
(stable web store build is working fine)
1 :- setting ui is completely messed up in mobile. I had to use landscape mode in mobile to customize.

2 :- setting button in bottom right is invisible on mobile. It works but invisible.

Thank you. Since this is meant to be a desktop extension it isn't high priority, but given it seems possible to get it onto mobile now I will look into this probably after the next update. Thank you for the feedback!


Listing table rows INCLUDING soft deleted

Pretty much what the title says. I am trying to do the equivalent of User::withTrashed(), but I'm not being able to do so. How can I run a query_filter that captures both deleted AND not deleted? It seems that if I try to fetch i.e. only the deleted ones, the resulting query is overwritten anyway and never fetches the deleted ones.

Even if the list shows soft-deleted records, you can't open them up to view all their fields (let alone to undelete them) -- clicking the row results in a "create new" form instead.


GROUP BY column list order affects runtime

Qry1:
--takes 1min  (UI json shows "cumulativeUserMemory" : 3.3699831825E11,  "totalScheduledTime" : "19.26m","peakTaskTotalMemory" : "57267780B",       "info" : {
"@type" : "exchangeClientStatus",
"bufferedBytes" : 0,
"maxBufferedBytes" : 485348,
"averageBytesPerRequest" : 12007,
"successfulRequestsCount" : 1264,
"bufferedPages" : 0,
"noMoreLocations" : true,
"pageBufferClientStatuses" : [ ]
}
SELECT a,b,c,d,e,
sum(value_double) as value FROM s.t
WHERE d BETWEEN TIMESTAMP '2020-03-01' AND TIMESTAMP '2020-08-11'
AND data_set_name IN ('e,'t')
GROUP BY d,b,a,e,c

Qry2:
--takes 2mins (UI json shows "cumulativeUserMemory" : 1.058281153416E12,  "totalScheduledTime" : "29.80m","peakTaskTotalMemory" : "105336700B",
"info" : {
"@type" : "exchangeClientStatus",
"bufferedBytes" : 0,
"maxBufferedBytes" : 485211,
"averageBytesPerRequest" : 5506,
"successfulRequestsCount" : 2752,
"bufferedPages" : 0,
"noMoreLocations" : true,
"pageBufferClientStatuses" : [ ]
}
SELECT a,b,c,d,e,
sum(value_double) as value FROM s.t
WHERE d BETWEEN TIMESTAMP '2020-03-01' AND TIMESTAMP '2020-08-11'
AND data_set_name IN ('e,'t')
GROUP BY a,b,c,d,e

from slack:
"
Tao Wang Apr 3rd at 3:58 AM
It's possible to improve the performance of the GROUP BY function by carefully ordering a list of fields within the GROUP BY in an order of high cardinality  I am a little confused about this tip. Based on my understanding, since the group by is implemented by hash in Presto, there should not be difference between different group-by order. Could someone shed light on this? Thanks!
15 replies
findepi  6 months ago
Where this quote is from?
raunaqm  6 months ago
I think this is mentioned by Athena and Treasure data in tuning guide.
https://aws.amazon.com/blogs/big-data/top-10-performance-tuning-tips-for-amazon-athena/
https://support.treasuredata.com/hc/en-us/articles/360001450908-Presto-Performance-Tuning
Amazon Web ServicesAmazon Web Services
Top 10 Performance Tuning Tips for Amazon Athena | Amazon Web Services
This blog post has been translated into Japanese.  Amazon Athena is an interactive query service that makes it easy to analyze data stored in Amazon S3 using standard SQL. Athena is serverless, so there is no infrastructure to manage, and you pay only for the queries that you run. Athena is easy to use. Simply […]
Mar 24th, 2017 (31 kB)
https://d2908q01vomqb2.cloudfront.net/b6692ea5df920cad691c20319a6fffd7a4a766b8/2017/11/16/Top10.png
Arm Treasure DataArm Treasure Data
Presto Performance Tuning
We have a new docs home, for this page visit our new documentation site!                             Prerequisites Basic knowledge of Arm Treasure Data. Basic knowledge of Presto query engine. ... (7 kB)
https://theme.zdassets.com/theme_assets/766587/78b016adfce27707ee53e066d510017a2788bdc9.png
findepi  6 months ago
I don’t know why the wrote so. Maybe that was based on some old version?
@Tao Wang in general — this certainly doesn’t matter, as you pointed out, everything is hash-based
in a less general case, when the data is already partitioned by a subset of columns, we do not repartition it.
but that doesn’t depend on the order of columns in GROUP BY either.
Tao Wang  6 months ago
Thanks @findepi and @raunaqm! I did see this quote from arm treasure data .  The reason i have this question is i have large performance difference with different order. Below are the same queries with different group by order. (edited)
Tao Wang  6 months ago
-- take about 10+ minutes
select
player_id_type,
game_id_type,
platform,
release_type,
source_type,
country,
mode_type,
game_mode,
game_type,
game_difficulty,
game_map,
is_spender,
is_new_player,
subscription_type,
event_source,
cast(udf_enhanced_approx_set(player_id, cast(0.0115 as double)) as varbinary) as active_users_sketch,
sum(event_count) as event_count,
sum(session_duration_sum) as session_duration_sum,
dt,
pgroup,
game_id
from ${db}.${table} where dt='2020-03-16'
group by 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,19,20,21;
Tao Wang  6 months ago
-- take less than 3 minutes
select
player_id_type,
game_id_type,
platform,
release_type,
source_type,
country,
mode_type,
game_mode,
game_type,
game_difficulty,
game_map,
is_spender,
is_new_player,
subscription_type,
event_source,
cast(udf_enhanced_approx_set(player_id, cast(0.0115 as double)) as varbinary) as active_users_sketch,
sum(event_count) as event_count,
sum(session_duration_sum) as session_duration_sum,
dt,
pgroup,
game_id
from ${db}.${table} where dt='2020-03-16'
group by 13,1,2,3,4,5,6,7,8,9,10,11,12,14,15,19,20,21;
findepi  6 months ago
can you compare EXPLAIN plans for different orders and check what is the distribution (what columns are hashed) of indivudal fragments?
findepi  6 months ago
since there is only TS + GROUP BY, there shouldn’t be any edge cases.
but, the difference can be attributed to equals invocations when finding values within bucket in memory
(let me checck) (edited)
Tao Wang  6 months ago
Is it possible related to the hash collision?
findepi  6 months ago
https://github.com/prestosql/presto/blob/fd5f4988df52dd49be6e95b681c0b87481ef4e4f/presto-main/src/main/java/io/prestosql/operator/MultiChannelGroupByHash.java#L286
presto-main/src/main/java/io/prestosql/operator/MultiChannelGroupByHash.java:286
if (positionNotDistinctFromCurrentRow(groupAddressByHash[hashPosition], hashPosition, position, page, (byte) rawHash, channels)) {
https://github.com/prestosql/presto|prestosql/prestoprestosql/presto | Added by GitHub
findepi  6 months ago
it’s not only about hash collisions per se (they are rare, if values are inequal), but it’s hash % size , to effective collisions are more frequent
that should be filtered out by the rawHashByHashPosition[hashPosition] != rawHash in that method… (edited)
findepi  6 months ago
I think there is something i am not seeing. @dain do you recall?
Tao Wang  6 months ago
Thanks @findepi! The EXPLAIN plans are pretty the same except the different group-by column. Looking forward to dain's reply. Thanks all.
findepi  6 months ago
different group-by column
just in different order, right?
Tao Wang  6 months ago
yes, you are right. just in different order
"

Maybe because of the combine_hash function when calculating the hash of columns in the GROUP BY clause.  According to the source code, the hash value may overflow after the left value multiple 31.
    @ScalarFunction(value = "combine_hash", hidden = true)
    @SqlType(StandardTypes.BIGINT)
    public static long getHash(@SqlType(StandardTypes.BIGINT) long previousHashValue, @SqlType(StandardTypes.BIGINT) long value)
    {
        return (31 * previousHashValue + value);
    }

@ssquan good point. I guess this doesn't work good e.g. when the number of worker nodes is multiple of 31.
Other scenarios?

In explain analyze verbose there should be hash collision stats for aggregation operator. Could you attach such plans?

Collisions avg.: 2532.00 (17882.48% est.), Collisions std.dev.: 0.00%
vs
Collisions avg.: 2534.00 (17896.61% est.), Collisions std.dev.: 0.00%

17882.48% is a high number. But it happens in both cases. Can you consistently reproduce perf difference in both scenarios? Maybe it's mostly executing on sing node, which has high perf variablity.

consistently different number of collisions based on order of the columns


Change allsky logging to ensure logs only written to allsky log files

Modified the services and rsyslog config to write to specific log files. This will prevent the AllSky log message sbeing duplicated in syslog and messages.
Log rotation has remained the same

Cool.  Thanks for doing this. Alex.  It will save a TON of writes to the SD card.


Can't use it with helmet.js.

Uncaught (in promise) EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' *.jsdelivr.net *.google.com *.googleapis.com *.unpkg.com *.gstatic.com *.googletagmanager.com *.gstatic.com *.jquery.com *.stripe.com 'nonce-T4L0hhBgQhfH42hznQgJeb=='".

    at Function (<anonymous>)
    at Object.<anonymous> (devtools-detector.js:1853:34)
    at devtools-detector.js:1833:29
    at Object.next (devtools-detector.js:1841:19)
    at devtools-detector.js:1749:43
    at new Promise (<anonymous>)
    at o (devtools-detector.js:1723:20)
    at Object.isOpen (devtools-detector.js:1848:20)
    at t.<anonymous> (devtools-detector.js:785:48)
    at devtools-detector.js:702:29

That's the error I get because I want to prevent eval() on my site, using helmet.js. But it seems like the script uses eval. Am I forced to disable my protection measures to use this script? Is there an alternative.

Fixed, please install the latest version.


fix: int64 vs float64 mismatch in SELECT floor(i)

 --- FAIL: TestQueriesSimple/SELECT_floor(i),_s_FROM_mytable_mt_ORDER_BY_floor(i)_DESC (0.09s)
        /Users/shannon/code/myduckserver/evaluation.go:726:
            	Error Trace:	/Users/shannon/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.2-0.20240815142344-761713e36043/enginetest/evaluation.go:726
            	            				/Users/shannon/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.2-0.20240815142344-761713e36043/enginetest/evaluation.go:357
            	Error:      	Not equal:
            	            	expected: []sql.Row{sql.Row{3, "third row"}, sql.Row{2, "second row"}, sql.Row{1, "first row"}}
            	            	actual  : []sql.Row{sql.Row{3, "third row"}, sql.Row{2, "second row"}, sql.Row{1, "first row"}}

            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -2,3 +2,3 @@
            	            	  (sql.Row) (len=2) {
            	            	-  (int64) 3,
            	            	+  (float64) 3,
            	            	   (string) (len=9) "third row"
            	            	@@ -6,3 +6,3 @@
            	            	  (sql.Row) (len=2) {
            	            	-  (int64) 2,
            	            	+  (float64) 2,
            	            	   (string) (len=10) "second row"
            	            	@@ -10,3 +10,3 @@
            	            	  (sql.Row) (len=2) {
            	            	-  (int64) 1,
            	            	+  (float64) 1,
            	            	   (string) (len=9) "first row"
            	Test:       	TestQueriesSimple/SELECT_floor(i),_s_FROM_mytable_mt_ORDER_BY_floor(i)_DESC
            	Messages:   	Unexpected result for query SELECT floor(i), s FROM mytable mt ORDER BY floor(i) DESC

“DuckDB and MySQL’s SUM and FLOOR functions have different return rules for different types, as follows:”
duckdb:
sum(DECIMAL) -> DECIMAL
sum(SMALLINT) -> HUGEINT
sum(INTEGER) -> HUGEINT
sum(BIGINT) -> HUGEINT
sum(HUGEINT) -> HUGEINT
sum(DOUBLE) -> DOUBLE
floor(FLOAT) -> FLOAT
floor(DOUBLE) -> DOUBLE
floor(DECIMAL) -> DECIMAL
mysql:
sum(integer/DECIMAL) -> DECIMAL
sum(float/double) -> double
floor() -> int
i have already changed hugeint to int64. may we need to make any other modifications, and is it acceptable to tolerate some incompatibilities?”

“DuckDB and MySQL’s SUM and FLOOR functions have different return rules for different types, as follows:” duckdb: sum(DECIMAL) -> DECIMAL sum(SMALLINT) -> HUGEINT sum(INTEGER) -> HUGEINT sum(BIGINT) -> HUGEINT sum(HUGEINT) -> HUGEINT sum(DOUBLE) -> DOUBLE floor(FLOAT) -> FLOAT floor(DOUBLE) -> DOUBLE floor(DECIMAL) -> DECIMAL
mysql: sum(integer/DECIMAL) -> DECIMAL sum(float/double) -> double floor() -> int
i have already changed hugeint to int64. may we need to make any other modifications, and is it acceptable to tolerate some incompatibilities?”

welcome back!


Is it possible to update document that not exist?

I use Precondition.None and it throw NullReferenceException
transaction.Update(document,dictionary,Precondition.None);

I think this is a bug because what will be usage of Precondition.None
But then I wonder if it really intended. Is it the limitation that we could not really update document that not yet exist?
If it really be the case I would like to have Precondition.None be deprecated because it has no usage. But if it was a bug in the library then I would like to have it fix real soon

Okay, it turned out to be a simple bug. Yes, it's possible to use Update with a document that doesn't already exist, and I now have an integration test for that.
The bug will be fixed by #2481. It's a trivial fix, but I'll want to get #2480 merged as well before doing a release - and I wouldn't want to do a release on a Friday anyway. Hopefully this will mean a new release on Monday.

Apologies for the delay in the release - I tagged it, but apparently forgot to trigger a build. This is now available in v1.0.0-beta10.


fix: bump crypto package to fix security vulnerability

Updated Crypto package to latest version

@slayerjain could u pls go through this PR and check if anything else needs to be done!

@khanjasir90 Thanks for the pr. We follow semantic commits - https://www.conventionalcommits.org/en/v1.0.0/
For now, I'm updating your title and will use it.

@slayerjain thanks! will surely follow the semantics from next time :).


Markdown not supported within a Card attachment

OS: Windows
OS Version: 10.15042.3
Emulator Version: 3.5.25
SDK: C# v3.5.5
The following response from a bot:
"attachments": [
    {
      "contentType": "application/vnd.microsoft.card.thumbnail",
      "content": {
        "title": "[GeekTrainer](https://github.com/GeekTrainer)",
        "subtitle": "[Click to view](https://github.com/GeekTrainer)",
        "text": "[Click to view](https://github.com/GeekTrainer)",
        "images": [
          {
            "url": "https://avatars2.githubusercontent.com/u/6109729?v=3"
          }
        ]
      }
    }

results in the markdown-written text, subtitle, and title fields rendered as plain text in the emulator:

The same is also true of HeroCard attachments:
"attachments": [
    {
      "contentType": "application/vnd.microsoft.card.hero",
      "content": {
        "title": "[GeekTrainer](https://github.com/GeekTrainer)",
        "subtitle": "[Click to view](https://github.com/GeekTrainer)",
        "text": "[Click to view](https://github.com/GeekTrainer)",
        "images": [
          {
            "url": "https://avatars2.githubusercontent.com/u/6109729?v=3"
          }
        ]
      }
    }

Hi @brandonh-msft This sounds like an issue of the WebChat control itself. Could you open an issue for it there? (https://github.com/Microsoft/BotFramework-WebChat)


Fixed section header numbering not resetting

Fix for issue #18
Haven't really tested this outside of checking if it actually fixed the problem.

Thank your for your PR.


High DPI Support

Hello,
an option to scale the size of menu icons and text to 1.5 or 2.0 would be nice if added in future updates :smile:

Scaling of just the menue should be possible
needs to be implemented here: https://github.com/Torridity/dsworkbench/blob/master/Core/src/main/java/de/tor/tribes/ui/RibbonLayoutConstants.java#L37
Maybe I'll change the check box into a slider in next weeks

Implemented in beta
For testing here is a build version:
https://github.com/extremeCrazyCoder/dsworkbench/releases/tag/3.73-pre-2


GitHub token scanning api view

Fixes #6051
High level description to ease review:
(Assuming you're familiar with the ticket).
The view
In this PR, we create a view (github_disclose_token) which:

Is a json POST API view meant to be called by GitHub
Returns either 204 no content or 40x and a json error payload
Expects a json body and 2 custom headers

This view uses 2 main components described below.
GitHubTokenScanningPayloadVerifyService
This service validates that the call originates from GitHub by doing the following:

A request to GitHub meta API & parsing the result to retrieve a public key, given a fingerprint
Check that the provided signature of the json payload of the request is valid given this public key

TokenLeakAnalyzer
This part is voluntarily independant from GitHub
The request JSON payload contains leaked tokens to analyse. Each token might either be a string of the form pypi-xxx or a b64 encoded form of __token__:pypi-xxx.

We extract the token from whichever form it is
We check if the token exists in the DB and it's signature is correct
If so we invalidate it
And send a mail to the user

The rest

We needed a new function check_if_macaroon_exists in the macaroon service (& associated tests)
There's a new email function send_password_compromised_email_leak and the associated email
There's a FAQ page

TODO:

[ ] Get some answers regarding:

[ ] How to cache things?
[x] Do we have a github token, and if not, would we plan to get one to avoid IP based rate limiting? Well, I've implemented it, but it's optionnal
[ ] When things go bad, how should we log?
[ ] How to check multiple headers in the view config predicates?

[x] tests:

[x] GitHubTokenScanningPayloadVerifyService
[x] NullGitHubTokenScanningPayloadVerifyService
[x] check_if_macaroon_exists
[x] TokenLeakAnalyzer
[x] github_disclose_token view
[x] send_password_compromised_email_leak

[x] FAQ in the doc
[x] Get an answer from GitHub and implement the signature crypto check
[x] Test that it works on the dev env

Hi @ewjoachim https://github.com/pypa/warehouse/blob/master/warehouse/email/ses/views.py has a lot of good reference that may help answer many of your open questions.

Ah, I'll look at this, thank you a lot.

I'm not sure I understand why the MessageVerifier is implemented as a class in /utils rather than as a service. I believe the architecture of what I'm trying to achieve is indeed very similar, so I'm enclined to do the same with a GitHubTokenScanningRequestVerifier, but I'm not sure I really understand why this should not be a service ?

Ok, it still misses tests & docs, but it should start being readable.
I'm interested in pointers for the remaining todos, feedback if my code style was right (escpecially if I need to redo large parts of the code, before I start writing all the tests) and maybe wording advice for the email.

Services are for things where you might realistically want to replace the implementation with something else at runtime. So the code that interacts with MessageVerifier is never going to want to use anything but a MessageVerifier, it makes no sense to make it swapable because that code is so directly tied to SES that there's not really a second implementation that makes sense.
However something like... where we store file objects is something that we might absolutely want to swap at runtime (and we do, we use different storage engines at runtime versus at development time) so it makes perfect sense for that to be a service. Another example is the HIBP code, one option when we were developing that was taking the HIBP data and running our own copy locally, using a service let us make that a more feasible option with minimal changes to the code if we ever decide to go that route.

It feels like we could have a reason to swap the fetching and signature verification part at run or test time (for example for a version that would not contact github and work with local keys and key ids).
On the other hand, the token check, revocation and user warning code seems pretty much straightfoward.
Is that to say I should have implemented this a a 3 parts:

Payload parsing & basic checks, not as a service
Signature check (including contacting github to check the public key), as a service
Examining tokens and revoking the valid ones, not as a service

?
Side note: I've tried to implement part 3 completely independently from GitHub, with the idea that we may want to have revocation protocols from other services (google ? manual with a rate limit ? Who knows ?), but whatever the origin of the disclosure is, it doesn't mean the implementation would change, so this would still say "not a service" if I understand correctly.
So, shall I change it all ?

So, shall I change it all ?

I've done that.
New iteration, far less # TODOs. Next step, get an answer from GitHub, and write some tests.

Missing coverage:
warehouse/accounts/services.py                      335      1     88      0    99%   805
warehouse/accounts/utils.py                          67     39      8      0    40%   37, 43-47, 82-104, 111, 115-146
warehouse/accounts/views.py                         284     19     90      0    94%   731-765
warehouse/email/__init__.py                          72      1     10      0    99%   174

Still a few open questions for which I'll need someone's help though (see in the PR message at the top)

Ok, I'm ready for review. I've been told the PR is likely too big to be easily review, which I would totally understand, but then I need suggestion on how to break it up into smaller ones.
Also, there are still unanswered questions regarding cache, logging and Pyramid's header predicate (see at the top in the todo-list)

Rebased, fixed conflict.
@ewdurbin / @dstufft  Do you think you might have some time to make a first round of review ? Is there something I can do to make it easier to you ?

Hey 👋 ! Crazy month... Any chance you could start reviewing or give me clues on what I should do to help this PR be reviewed ?
Meanwhile, I'll rebase and fix conflicts :)

Should there be a way to quickly enable/disable/silent the feature in case it doesn't work as expected ?

Hey there :) I hope it's ok if I ping this PR every month or so :) Tell me otherwise !
Again, if there is anything that needs to be done before reviewing, please tell me :)

Okay ! @di I've addressed all your comments except the one about switching to celery. In dedicated commits, I made sure not to merge them with previous commits so that you can continue the review from where you were. When we're done, I can squash if you want.
What's left:

[ ] I've avoided re-doing the translations (messages.pot) so far because with my setup, it's a bit complicated, but it's a generated file, I'll get it right when the rest is ok.
[ ] The tests. I haven't touched the test after the big changes. I hope it will mostly be about moving tests more than writing tests, but we'll see
[ ] and the celery part (and this time, it will involve writing new tests)

Am I missing something ? I'll be continuing to work on that, when I find some time :)

Okay !
Took some time but I think we're at the end of the road.

All the review comments were take into account
Tests are at 100% coverage again
Translations rebuilt
Linters pass
All TODOs are gone. All checkboxes in this PR (which is approaching 100 comments 🎉) are checked.

Note: I haven't been able to test the whole thing for real, this would require GitHub calling us and I don't know how to that.
Once review is good, I can redo the commits if you want, fixing up and squashing things for a cleaner history, but at this point, I have to admit that I'll be glad if we could call it a day.

Response from GitHub about some of the open questions here:

Are we expected to use tokens of some sort to call https://api.github.com/meta/public_keys/secret_scanning to avoid rate-limiting or is this URL exempt of rate-limiting ? If this URL is NOT exempt, can you confirm that, without a token, our ability to get a response will depend on how many calls there were recently, from the same IP address ? If we'd rather avoid using a personal access token here, what would be the alternatives ? Or should we use a personal access token from a dedicated account ?

Authenticated requests (associated with a user) have a higher rate limit than unauthenticated ones. AFAIK, Secret scanning does not have any special rate limiting rules. The general rules defined here should apply: https://docs.github.com/en/rest/overview/resources-in-the-rest-api#rate-limiting
The public key used to sign a hook is provided, and these keys do not change too often. We recommend caching the key locally, so it only needs to be fetched if not present in the cache, or when the key changes.

When processing GitHub's HTTP call, upon detection of a token leak, is there any change of behaviour on GitHub side that depends on the status code or body of the secret ? Or is the response completely ignored ? What if the response is 5XX, will the call be tried again later ?

We do retry requests that do not receive a 2xx HTTP success response. Apart from that, the response code does not influence behaviour. Specific types of secrets may have custom processing applied to them after they are detected.

I think my preference would be to not use a PAT here and instead do some light caching (via Redis).

Once review is good, I can redo the commits if you want, fixing up and squashing things for a cleaner history, but at this point, I have to admit that I'll be glad if we could call it a day.

I don't think this is necessary! We'll just squash this all together into one commit when merging anyways.
Really appreciate you seeing this through @ewjoachim! I think we're nearly there 😅

I think my preference would be to not use a PAT here and instead do some light caching (via Redis).

I understand how easier for deployment and maintenance this will be, and in the end you'll be the judge. Just sharing my experience on the matter: it all depends on the deployment specifics and whether we'll be the only ones using the outbound IP. Without a PAT, we're limited to 60 calls / hour, for all the users of the same IP, which will include warehouse workers, but can also include e.g. many other unrelated tenants in the same public cloud, or this kind of stuff. If you're sure that we won't have to share our rate limit bucket, then depending on how many web workers we have, even an in-memory cache might suffice. If we're sharing the IP though, we're pretty much guaranteed that 95% calls will fail, so caching won't help.

Sure. I think that's a question @ewdurbin can probably answer better than I can.
I also just realized we have https://github.com/browntruck which we could use to generate a PAT (although I'm not sure who has control of that account right now)

If the cache is used and the key wasn't found, we should bust the cache and retry before dclaring it a failure. I missed that part.

If the cache is used and the key wasn't found, we should bust the cache and retry before declaring it a failure. I missed that part.

Done. Now the cache will be used if it exists and it's not too old, but if the key is not found in the cache, the "normal" way will be used. Also, added metrics for cache hits and misses to ease potential debugging.

I was under the impression that we were about to be ready. Is there anything more I could be doing ? @di ?

Rebases, conflicts solved, all good !

OK, one last small change to request: can you update this so that it's not required that GITHUB_TOKEN is set? I.e. if the env var is not present, this should just not attempt to include the Authorization header at all.

(Also, looks like linting is failing because we're using a newer version of black now, and the translations need regenerated)

OK, one last small change to request: can you update this so that it's not required that GITHUB_TOKEN is set? I.e. if the env var is not present, this should just not attempt to include the Authorization header at all.

Yep no problem :+1: (we did a full 360 from https://github.com/pypa/warehouse/pull/7124#discussion_r453686794 :tada: )

(Also, looks like linting is failing because we're using a newer version of black now, and the translations need regenerated)
I'll rerun black. Translations: I regenerated them completely, I'm surprised.

ok, make translations does not create diff, neither does make reformat. The api token is now optional.

Thanks @ewjoachim for all your work here!


Still using old 0.21.1 axios version

I'm submitting a...

[ ] Regression
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

This package locks on "axios": "0.21.1", while there are open CVEs for below 0.21.4.
Expected behavior

Either using latest, using axios as a peer dependency or upgrading to the latest non-vuln version.
Minimal reproduction of the problem with instructions

https://snyk.io/vuln/SNYK-JS-AXIOS-1579269
What is the motivation / use case for changing the behavior?

Environment

Nest version: X.Y.Z

this issue occurs with the latest version of this package at 0.0.1

For Tooling issues:
- Node version: XX
- Platform:

Others:

Upgrading from an older nest 7 to nest 8 system, I saw HttpModule was deprecated and followed the update instructions. But this package is below 1.0 and is using a locked dep too.

I know a couple of PRs have been opened already to bump the version up to 0.21.4 but I've submitted an alternative PR which does that and also loosens up the semver notation to allow for any future versions to satisfy this package's axios dependency. https://github.com/nestjs/axios/pull/149

Can we get this reviewed and merged (or #149) ASAP. This is critical for companies that need to stay within compliance in Snyk.

Any news about this? It's quite a blocker for a more than a week.

Any updates? its super critical...

Tracking this here https://github.com/nestjs/axios/pull/145


Hi im getting this error

could somebody help me whit this issue or error i dont know what to do.
PDO error when working with database: SQLSTATE[42S22]: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Invalid column name 'originalPassword'.

i find out it was the vote table and query script USE [Auction]
GO
/****** Object:  Table [dbo].[vote]    Script Date: 2022-11-12 09:06:27 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[vote](
[id] [int] IDENTITY(1,1) NOT NULL,
[name] char NULL,
[prize] [int] NULL,
[link] [text] NULL,
[image] [text] NULL,
CONSTRAINT [PK_vote] PRIMARY KEY CLUSTERED
(
[id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO
sorry im new here, but i solve the problem there is no issue on this script now.

here is the new issue i have im trying to edit item mall item on my webpage but it says.
PDO error when working with database: SQLSTATE[42000]: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Incorrect syntax near 's'.
here is the script for sql query
USE [Auction]
GO
/****** Object:  Table [dbo].[ItemMall]    Script Date: 2022-11-12 09:00:01 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[ItemMall](
[ItemName] char NOT NULL,
[ItemPrice] [int] NOT NULL,
[ItemDesc] [text] NOT NULL,
[Quota] [int] NOT NULL,
[Icon] char NOT NULL,
[cType] char NOT NULL,
[ItemID] [int] NOT NULL,
[quantity] [int] NOT NULL,
[category] char NOT NULL,
[MallID] [int] IDENTITY(1,1) NOT NULL
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO

I see the create table queries, but what is the PHP application trying to do when you get these errors? Is it executing the create table queries, or is it trying to insert/select from said tables?

Close due to inactivity


Unsupported hash type ripemd160

Tested on OpenSSL 3.0.2, Python 3.10, Linux Ubuntu 22.04 LTS.
Hashlib raises ValueError: unsupported hash type ripemd160, in ripemd160_sha256 function in crypto.py. Found the reason for the issue here.

Hashlib uses OpenSSL for ripemd160 and apparently OpenSSL disabled some older crypto algos around version 3.0 in November 2021. All the functions are still there but require manual enabling. See issue 16994 of OpenSSL github project for details.

The stackoverflow answer details how to enable legacy crypto algorithms, but I think a default solution is needed to use ripemd160.

How does this impact BitCash?

BitCash uses the hash160 (ripemd160_sha256) function to convert public keys to cashaddress and public key hashes for output locking script of P2PKH outputs. This affects the primary functionality of BitCash as a P2PKH wallet.
hash160 uses the hashlib of python, which uses the OpenSSL implementation of ripemd160 which no longer works by default (as tested on OpenSSL 3.0.2, Python 3.10, Linux Ubuntu 22.04 LTS).

I am unable to reproduce this.
Can you send a minimal reproducible example?

from hashlib import new, sha256
new("ripemd160", sha256(b"a").digest()).digest()

I tested this on OpenSSL 3.0.2, Python 3.10.6.
Apparently OpenSSL had already marked ripemd160 as legacy algorithm. Do check if you have already enabled legacy_sect as noted here, if you are on the same version of OpenSSL.
This is what my default openssl.cnf file looked like. (only showing relevant options)

openssl_conf = openssl_init
[openssl_init]
providers = provider_sect
[provider_sect]
default = default_sect
[default_sect]
# activate = 1

This led the above python code to give "ValueError: unsupported hash type ripemd160"
After when I enabled legacy algorithms, as given here, the ripemd160 code gave no errors.

from hashlib import new, sha256
new("ripemd160", sha256(b"a").digest()).digest()

This works on my environment (macOS, Python 3.10.6, LibreSSL 3.3.6).
In [1]: from hashlib import new, sha256
   ...: new("ripemd160", sha256(b"a").digest()).digest()
Out[1]: b'\x99CU\x19\x9eQo\xf7lO\xa4\xaa\xb3\x937\xb9\xd8L\xf1+'

I can try to reproduce in a different environment.
What would be a way to solve this within BitCash? And how much work would that involve?

It seems its a OpenSSl only problem. Other bitcoin libraries have dealt with the same: https://github.com/bitcoin/bitcoin/issues/23710.
Most straightforward way would be to code python native ripemd160 code, as done here

Seems pretty straightforward:
def ripemd160(data):
    """Compute the RIPEMD-160 hash of data."""
    # Initialize state.
    state = (0x67452301, 0xefcdab89, 0x98badcfe, 0x10325476, 0xc3d2e1f0)
    # Process full 64-byte blocks in the input.
    for b in range(len(data) >> 6):
        state = compress(*state, data[64*b:64*(b+1)])
    # Construct final blocks (with padding and size).
    pad = b"\x80" + b"\x00" * ((119 - len(data)) & 63)
    fin = data[len(data) & ~63:] + pad + (8 * len(data)).to_bytes(8, 'little')
    # Process final blocks.
    for b in range(len(fin) >> 6):
        state = compress(*state, fin[64*b:64*(b+1)])
    # Produce output.
    return b"".join((h & 0xffffffff).to_bytes(4, 'little') for h in state)

Would you be willing to submit a PR for that?

I am able to reproduce. Looking forward to merging this change 👍🏻


added optional field  to web pixel RuntimeAPI

Background
(Provide a link to any relevant issues AND provide a TLDR of the issue in this pull request)
ISSUE: https://github.com/Shopify/ce-customer-behaviour/issues/1461
we must rename configuration to settings in the Runtime API.
Solution
Adding in an optional settings property to support the transition until we are confident in the solution downstream
🎩

...

Checklist

[ ] I have :tophat:'d these changes
[ ] I have updated relevant documentation

LGTM, assuming we will remove configuration and change settings? to settings after we ship the web-pixels-manager change

yes, this is so that we can release the settings and update in the references in the gem. Afterwards we will remove the configuration field


Update bump_sdk workflow to edit conda-env.yml

bump sdk workflow updates the git snippet which pulls in new using info, but not the conda env meaning the api reference doens't change

https://github.com/ASFHyP3/ASFHyP3/blob/main/conda-env.yml#L14


fix example-robot-data depenency by a small change on the API

Hi,
This PR is in response to this issue https://github.com/Gepetto/aig/issues/19
I did an update in the API to prevent a dependency to the example-robot-data package in the main API.
Only the unit-tests depends on it.
In exchange to that the user need to provide the path to the robot_description package.
I believe it's a small cost to pay, but let me know if you think it's not the best way.

It seems good to me.
But, It seems to be mandatory for the user to include example-robot-data in his application. Is there a way for the user to not use the example-robot-data?

Maybe we can ask the user to provide a pinocchio model directly ? This way, example-robot-data could provide a pinocchio model from a name in the example.
For now, example-robot-data only has this feature in python, but adding it in C++ would be easy.

I was going to comment to ask to add a way to parse the urdf from an xml string but actually it's already there but not documented : https://github.com/Gepetto/aig/blob/master/src/biped_ig.cpp#L77
With this I will use aig without example robot data at all in my applications. But maybe we could improve a little the api on this part to add clarity, eg. add an optionnal member in the setting struct "urdf_string". If provided it parse this as xml, otherwise it check for the urdf path in initialize().

It seems good to me. But, It seems to be mandatory for the user to include example-robot-data in his application. Is there a way for the user to not use the example-robot-data?

example-robot-data is not needed, you can easily provide the path to the model of the robot through ros for example.
or any other package manager who will tell you where your urdf is.
So I believe the current implementation is not so bad.

Maybe we can ask the user to provide a pinocchio model directly ? This way, example-robot-data could provide a pinocchio model from a name in the example.
For now, example-robot-data only has this feature in python, but adding it in C++ would be easy.

I could ask for a pinocchio model but I do not like the API to contain other stuff than "std" and "Eigen" materials
This makes the application depending only on aig and implicitly only on pinocchio.
So it's by design that I did this. If you guys have a strong opinion I can change it.

In anycase I believe that some work needs to be done in the init of these settings.
One should only provide the root_link_name, end_effector_link_name for each limb and probe the model for existance and the intermediate links.
WDYT?

I think this can be merged when @pFernbach comment will be handled :)

I was going to comment to ask to add a way to parse the urdf from an xml string but actually it's already there but not documented : https://github.com/Gepetto/aig/blob/master/src/biped_ig.cpp#L77
With this I will use aig without example robot data at all in my applications. But maybe we could improve a little the api on this part to add clarity, eg. add an optionnal member in the setting struct "urdf_string". If provided it parse this as xml, otherwise it check for the urdf path in initialize().

I have to say I think having to check 2 variables instead of one makes things confusing if you register both...
This is why I only created one.
I could just rename the variable urdf instead of urdf_path and srdf instead of srdf_path and then they can be both strings or file path. Then in initialize I kep the same code.
WDYT @pFernbach ?

Yes I agree with this, it makes more sense than now. As long as it's documented it's good.

@NahuelVilla @pFernbach
Before merging, because this will break some api so be careful we need a small PR in pal_lip_mpc main and devel?


Modular page: CSS grouped assets doesn't work like JS

From my modular page, I want to add JS at the bottom of BODY and add extra CSS at the bottom of HEAD. It is successful so far with JS but extra CSS. Here is my HTML structure:

        {% include 'partials/head.html.twig' %}
        {% block top %}
            {{ assets.css('top') }}
        {% endblock %}

        {% block content %}{% endblock %}
        {% block bottom %}
            {{ assets.js('bottom') }}
        {% endblock %}

Here is what inside my modular page which is an image slideshow which requires its own JS files and CSS files:

    {% for image in page.header.images %}

    {% endfor %}

{% do assets.addCss('theme://css/slideshow.css', { 'group': 'top' }) %}
{% do assets.addJs('theme://js/slideshow.js', {'group': 'bottom'}) %}

{% do assets.addInlineJS('
(function(){
    // Slideshow configuration here
', {'group':'bottom'}) %}

slideshow.js and inline JS are added to bottom block successfully, but slideshow.css is not added to top block.
Can we do this with CSS? Or do I do something wrong here? Thanks!

CSS is only valid in the  tag sonteallly it makes no sense to support groups.

In https://learn.getgrav.org/themes/asset-manager#grouped-assets group is supported for both JS and CSS. Maybe I misunderstand it?

Oh sorry, misread your request. Thought you were asking about CSS at the bottom of the page. You should be able to do what you want by adding appropriate blocks in your theme.  However, this is not trivial for modular pages.  This is because that twig is processed during the content render, and after it's cached, it's not reprocessed.  Also You would have to be able to extend the base template in order to add the twig.  For this reason, you should put the CSS/JS in the top level modular page, not the modular sub-pages.

I see. Thank you for the info!


Typos

https://github.com/RalfEggert/zend-expressive-tutorial/blob/4b37066506921aa1fbcac508dcc7c413c657e747/doc/book/part3.md#update-the-album-configuration has a typo. "applicaition" should be "application"
https://github.com/RalfEggert/zend-expressive-tutorial/blob/part6/doc/book/part4.md#create-album-form-create-action has a confusing sentence "It does not handle the form when send."
https://github.com/RalfEggert/zend-expressive-tutorial/blob/part6/doc/book/part5.md#add-delete-action-to-show-form has a typo in the second paragraph. "from" should be "form".
https://github.com/RalfEggert/zend-expressive-tutorial/blob/part6/doc/book/part5.md#add-delete-action-for-form-handling has a typo. "delete_album_yes was send" should be "delete_album_yes was sent".

Thanks. I will fix that soon.

Thanks again. I just fixed all typos.


How to save video processing on detectnet

Hello, I am using this code so that my model can process video instead of just images on the detectnet, it works but I can only see it while processing. How do I get him to record this video with the bounding boxes and results in an .mp4 file?

Hi @Sgomes14843 , add videos/my_output.mp4 to the end of your command line.

Hi

, first thanks for responding.
I tried this but the output is like a text file instead of video. Do you know what it might be?

Hi @Sgomes14843 , add videos/my_output.mp4 to the end of your command line.
Hi, first thanks for responding.
I tried this but the output is like a text file instead of video. Do you know what it might be?

Hi @Sgomes14843 , add videos/my_output.mp4 to the end of your command line.

Hi, first thanks for responding.
I tried this but the output is like a text file instead of video. Do you know what it might be?

I think it loos like the text file icon shortly after the video has been created, before Ubuntu recognizes it.
If you try to open the video in a video player, does it play?

I think it loos like the text file icon shortly after the video has been created, before Ubuntu recognizes it.
If you try to open the video in a video player, does it play?

If you open it in the video player it doesn't give and gives this error and if it opens as a notepad it is empty, there is nothing in the file

Can you provide the command line that you ran along with the terminal output?
Did the process shutdown gracefully when you either closed the GUI window or pressed Ctrl+C?

The command line:
python3 detectnet.py --model=models/cars/ssd-mobilenet.onnx --labels=models/cars/labels.txt \           --input-blob=input_0 --output-cvg=scores --output-bbox=boxes \             videos/f1.mp4   videos/my_output.mp4
Yes the process closed properly when i close the GUI or press CRTL + C
Here's what it says on the terminal when I close the GUI or press CTRL + C

Hmm, well it appears to be running correctly.  If you run ls -ll videos/my_output.mp4, does it also show a size of 0 bytes?

Yes

I can't seem to tell why it's doing that, sorry.   Can you try removing the backslashes (\) from your command line to see if that makes a difference:
python3 detectnet.py --model=models/cars/ssd-mobilenet.onnx --labels=models/cars/labels.txt  --input-blob=input_0 --output-cvg=scores --output-bbox=boxes  videos/f1.mp4 videos/my_output.mp4

Also can you try running the C++ version of the application?
detectnet --model=models/cars/ssd-mobilenet.onnx --labels=models/cars/labels.txt  --input-blob=input_0 --output-cvg=scores --output-bbox=boxes  videos/f1.mp4 videos/my_output.mp4

Failing that, please post the full text log of the programming running so I can look it over - thanks.

Unfortunately none of these lines of code worked.
How do i do the full text log of the programming running ?

You can either copy it from your terminal into a text file, or launch the program like this with | tee log.txt to write the log:
detectnet --model=models/cars/ssd-mobilenet.onnx --labels=models/cars/labels.txt  --input-blob=input_0 --output-cvg=scores --output-bbox=boxes  videos/f1.mp4 videos/my_output.mp4 | tee log.txt

Thanks for teaching how to do this, here's the log file
log.txt

Thank you, I didn't identify any errors that jumped out at me.   But maybe try saving a different video format - my_output.mkv instead of my_output.mp4.   Does that help?
Also, it occurred to me to try a more regular input resolution instead of 854x480, like 640x480 (just as a test):
detectnet --model=models/cars/ssd-mobilenet.onnx --labels=models/cars/labels.txt  --input-blob=input_0 --output-cvg=scores --output-bbox=boxes --input-width=640 --input-height=480 videos/f1.mp4 videos/my_output.mkv

It worked, yeah!!!
Thank you so much for all your help and the time you spent
And I tested it with .mp4 and it worked so the problem was the resolution.

Aha, ok - glad you got it working.  That is strange, is seems the encoder didn't like that resolution but didn't output any messages/warnings about it.  I will have to remember that...


Config constructor parameter

In Akka 2.4 the plugin may have a constructor with a Config parameter, which corresponds to the config section that the user has defined. So instead of reading config from hardcoded "cassandra-journal", we should use the config parameter.
CassandraJournal should be defined as:
class CassandraJournal(cfg: Config) extends AsyncWriteJournal with CassandraRecovery with CassandraConfigChecker with CassandraStatements {

  val config = new CassandraJournalConfig(cfg)

Same with CassandraSnapshotStore.
This makes it possible to use more than one Cassandra plugin and each can have its own configuration.

Thanks for the hint @patriknw. @gvolpe, are you interested in making a PR? I currently don't have the bandwidth to work on that issue.

I can do that

@matlockx awesome, thanks a lot!

@matlockx please change to lazy val session at the same time. Otherwise the actor can throw  from constructor if cassandra is not running, and that will stop the actor.

@Patriknw Ok,did it. Pull request comes later. I've written a test which configures two actors to use same persistenceId but different tables, then saves some messages, recovers the actors and checks whether they have not influenced themselves (comparing the last sequence number). Do you think that is enough or is there a better way to test it?

Sounds like a great test, thanks.

Can we close this based on #116  and #121 c


[FLINK-1376] [runtime] Add proper shared slot release in case of a fatal TaskManager failure for 0.8 branch

This PR introduces SharedSlots as being a special Slot type and as such being released properly in case an Instance has been marked dead. This fixes the problem that a dead instance, which has not been shutdown properly, causes a job not being removed properly from the system, because it is not aware of the SubSlots.
Adds test cases where only the heartbeat thread of TaskManager is killed.
Except for the test cases, this is basically the same PR as #317 just rebased on the 0.8 branch.

I've merged it to the 0.8 branch. I forgot to close the PR, could you do it manually?
Thank you.


Generate package under src folder

As for any well organized project/library, it would be good to have the generated classes/files located under the src folder.
We would then have:
├── src
    ├── EnumType
        ├── << FILES >>
    ├── ServiceType
        ├── << FILES >>
    ├── StructType
        ├── << FILES >>
    ├── ClassMap.php
├── vendor
├── composer.json
├── composer.lock
└── tutorial.php

there is no way, how to disable this weird behavior
you should define src subfolder by this parameter, that could be default --destination="src/"
I am not abble to generate clases to Symfony standard directory structure
--destination="src/AppBundle/Soap"

This is partly due to several discussions from https://github.com/WsdlToPhp/PackageGenerator/issues/8 and https://github.com/WsdlToPhp/PackageGenerator/issues/17 that lead me to always use a src folder.
Nevertheless, the default destination can't be set to src/ as users don't always use the current directory (where you use the generator) as the destination folder. Personnally, I prefer to generate the package independently/outside from the main projet so I use the generated package as a vendor.
I also understand your position. Do you wish to make a pull request?

Hi, I found another sollution, but it is borring
php ./bin/wsdltophp.phar generate:package --urlorpath=http://xxxx/?singleWsdl --composer-name=name/soap --destination=./ --services-folder=AppBundle\\Soap\\ServiceType --enums-folder=AppBundle\\Soap\\EnumType --arrays-folder=AppBundle\\Soap\\ArrayType --structs-folder=AppBundle\\Entity --soapclient=AppBundle\\Soap\\Client --force

I don't see how this could work.
Are you on windows to be able to use backslashes in AppBundle\\Soap\\ServiceType. In addition, to be sure, the classes are still generated under the serc folder, right?
In any case, I don't understand why you couln't use the generated classes even if they are generated under the src folder.  Is there something I'm missing? Do you encounter autoloading issues? As you shouln't alter/modify the generated classes, I think there is no need to wonder where they are generated.

This command is tested in Windows
I am using folder src but in this way src/AppBundle/Soap/ServiceType; I am distributing Symfony Bundle, so I need this folder structure

could you try with the current develop branch using the command php console generate:package in order to tell me if it works as expected,


test(client): add integration runner

Includes and closes: https://github.com/prisma/prisma/pull/13169

Why

https://github.com/prisma/prisma/issues/13092
I was finding that the integration tests were running all at once and it was hard to tell jest to treat the integration differently without losing potential on the simple tests.

What
This PR excludes all of the tests in ./packages/client/tests/integration from the main test suite and introduces:
integration-runner.test.ts
This new file will readDir both the happy/ and errors/ tests and then perform sequential batching of n amount of tests.
Between each batch, a backoff is added so that we don't completely overwhelm other components such as the database.
How can I run them?
You should not need to change anything in your workflow, however, if you especially want to run the integration test's on their own you can run:
cd ./packages/client && pnpm run test integration

Findings
This shows my recorded stats of the time taken to run the client test suite before, with maxWorkers, and then after this addition.

Test Suites: 3 failed, 5 skipped, 181 passed, 184 of 189 total
Tests:       35 failed, 18 skipped, 717 passed, 770 total
Snapshots:   562 passed, 562 total
Time:        560.228 s
Ran all test suites.
 ELIFECYCLE  Test failed. See above for more details.

cd ./packages/client && pnpm run test integration
Could not run - Timed out!
Test Suites: 3 failed, 4 skipped, 142 passed, 145 of 149 total
Tests:       17 failed, 12 skipped, 554 passed, 583 total
Snapshots:   383 passed, 383 total
Time:        297.905 s
Ran all test suites matching /integration/i.
 ELIFECYCLE  Test failed. See above for more details.

Using maxWorkers

https://github.com/prisma/prisma/pull/13149#pullrequestreview-962094944

These run! However, the issue here is now that the entire test suite is running at 50% capacity when in reality we only want to limit the heavy load stuff(the integration tests).
cd ./packages/client && npx jest --maxWorkers="50%"
Pass - Slower as expected!
5 skipped, 184 passed, 184 of 189 total
Tests:       18 skipped, 752 passed, 770 total
Snapshots:   580 passed, 580 total
Time:        523.493 s, estimated 538 s

cd ./packages/client && npx jest integration --maxWorkers="50%"
Pass
Test Suites: 4 skipped, 145 passed, 145 of 149 total
Tests:       12 skipped, 571 passed, 583 total
Snapshots:   399 passed, 399 total
Time:        330.482 s
Ran all test suites matching /integration/i

After
cd ./packages/client && pnpm run test
Pass - 222.898 seconds faster
Test Suites: 1 skipped, 40 passed, 40 of 41 total
Tests:       6 skipped, 183 passed, 189 total
Snapshots:   181 passed, 181 total
Time:        300.595 s
Ran all test suites.

cd ./packages/client && pnpm run test integration
Pass - 83.634 seconds faster
integration/integration-runner
    √ happy (243661 ms)
    √ errors

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        246.848 s
Ran all test suites matching /integration/i.

Regressions
Some things we miss out on in this implementation, this is because we are now spawning Jest using exec. This means that the main Jest process doesn't have a full collective view of how many tests are run for example or the coverage of those files tested on inside the spawned process.

Jest Stats - You can still see all in the console but it won't be in one nice neat place like normal.
Code Coverage? - IDK yet standby

Included and closes: https://github.com/prisma/prisma/pull/13169

Findings

Yes, and this does help. I have added a section in the PR description Findings that shares my stats using maxWorkers.

Have you tried limiting maxWorkers in jest.config.js? This is not the same thing as what you're doing in this PR (which is also an interesting idea), but it might've solved the issue.

Yes, and this does help. I have added a section in the PR description Findings that shares my stats using maxWorkers.


Minimize Engine/AgentReconciler etcd requests

I have a cluster running 2 units. The engine makes the following requests to gather the relevant info for a single reconciliation:
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.002611 01384 reconciler.go:38] Polling Registry for actionable work
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.003835 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/job?consistent=true&recursive=true&sorted=true
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.004529 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/job?consistent=true&recursive=true&sorted=true: 200 OK
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.005956 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/unit/e5a03e40306d918f3c0f6eb056a8e62acc1822d8?consistent=true&recursive=true&sorted=false
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.006835 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/unit/e5a03e40306d918f3c0f6eb056a8e62acc1822d8?consistent=true&recursive=true&sorted=false: 200 OK
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.007362 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/unit/d9de011694dda111458a4fc9912ad629d86ad8af?consistent=true&recursive=true&sorted=false
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.008796 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/unit/d9de011694dda111458a4fc9912ad629d86ad8af?consistent=true&recursive=true&sorted=false: 200 OK
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.009337 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/job?consistent=true&recursive=true&sorted=true
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.010080 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/job?consistent=true&recursive=true&sorted=true: 200 OK
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.011552 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/state?consistent=true&recursive=true&sorted=false
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.012973 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/state?consistent=true&recursive=true&sorted=false: 200 OK
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.013626 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/states?consistent=true&recursive=true&sorted=false
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.015100 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/states?consistent=true&recursive=true&sorted=false: 200 OK
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.015385 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/machines?consistent=true&recursive=true&sorted=true
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.016772 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/machines?consistent=true&recursive=true&sorted=true: 200 OK
Aug 22 17:27:29 core-01 fleet[1384]: I0822 17:27:29.017024 01384 engine.go:71] Engine completed reconciliation in 14.414077ms

The agent ends up making a similarly-inefficient set of calls:
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.000151 01384 reconcile.go:68] AgentReconciler tick
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.001583 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/job?consistent=true&recursive=true&sorted=true
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.009265 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/job?consistent=true&recursive=true&sorted=true: 200 OK
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.010507 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/unit/e5a03e40306d918f3c0f6eb056a8e62acc1822d8?consistent=true&recursive=true&sorted=false
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.011013 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/unit/e5a03e40306d918f3c0f6eb056a8e62acc1822d8?consistent=true&recursive=true&sorted=false: 200 OK
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.012216 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/unit/d9de011694dda111458a4fc9912ad629d86ad8af?consistent=true&recursive=true&sorted=false
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.015528 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/unit/d9de011694dda111458a4fc9912ad629d86ad8af?consistent=true&recursive=true&sorted=false: 200 OK
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.018001 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/job?consistent=true&recursive=true&sorted=true
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.019370 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/job?consistent=true&recursive=true&sorted=true: 200 OK
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.019657 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/state?consistent=true&recursive=true&sorted=false
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.022429 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/state?consistent=true&recursive=true&sorted=false: 200 OK
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.022523 01384 client.go:353] etcd: sending HTTP request GET http://localhost:4001/v2/keys/_coreos.com/fleet/states?consistent=true&recursive=true&sorted=false
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.024085 01384 client.go:360] etcd: recv response from GET http://localhost:4001/v2/keys/_coreos.com/fleet/states?consistent=true&recursive=true&sorted=false: 200 OK
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.024225 01384 reconcile.go:265] Desired state "launched" matches current state of Job(foo.service), nothing to do
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.024233 01384 reconcile.go:265] Desired state "launched" matches current state of Job(ping.service), nothing to do
Aug 22 17:41:06 core-01 fleet[1384]: I0822 17:41:06.024240 01384 reconcile.go:44] AgentReconciler completed reconciliation in 22.798384ms

There are two problems that need fixing:

fleet gets each unit file individually - get all unit files with a single GET /unit?recursive=true
fleet calls  GET /job?recursive=true twice - this should just be done once

We have discovered this too while investigating coreos/fleet/issues/1163. Fixing 1 and 2 should take pressure from etcd while 2 would also reduce the number of JSON unmarshal calls.

@bcwaldon are you planning to work on optimisations like this for the next releases?

@denderello Not any time soon. Feel free to work on this if you have the time.

+1