[Storage]: Severe performance degradation observed using v12.0 *.aio BlobClient

Severe performance degradation observed using v12.0 *.aio BlobClient
Team...on a current project we are seeing some drastic performance degradations when attempting to use the *.aio libs from v12.0 of azure-storage-blob. It's quite possible we may be using the library incorrectly, but have not been able to find any documentation stating otherwise. The issue outlined below may in fact be a bug.
System Under Test

Repo: https://github.com/mjyeaney/CustomVisionSlidingWindowAPI
Linux App Service Plan
B2 App Service Plan (200 ACU, 3.5 GB RAM, A-series equiv), instance count = 1
Running Gunicorn -> Uvicorn (ASGI server) -> Quart (async REST fx)
Gunicorn worker count: CPU core count + 1
Using azure-storage-blob v.12.0, *.aio methods

Load Testing Environment

Located in same datacenter
Ds2v3 (2 CPU, 8GB RAM)
Tests run using Apache Bench (ab) utility

Tests
There are 2 endpoints that we're testing to ensure we don't have any unexpected regressions. The first is a simple JSON serialization using coroutines/await+async, and does NOT use the Azure storage SDK in any code path.
Running ab against this endpoint with n=1000, c=50, this is handling ~450 - 550 req/sec (sample output below), showing that even on limited dev/test hardware this is performing reasonably well:
Concurrency Level:      50
Time taken for tests:   1.842 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      330000 bytes
HTML transferred:       38000 bytes
Requests per second:    542.83 [#/sec] (mean)
Time per request:       92.110 [ms] (mean)
Time per request:       1.842 [ms] (mean, across all concurrent requests)
Transfer rate:          174.94 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        1    2   0.5      2       8
Processing:     4   89  57.0     80     318
Waiting:        4   89  57.0     80     318
Total:          5   91  57.1     82     321

Percentage of the requests served within a certain time (ms)
  50%     82
  66%    103
  75%    120
  80%    130
  90%    159
  95%    186
  98%    245
  99%    319
 100%    321 (longest request)

The second endpoint is simply downloading a VERY small JSON blob (~76 bytes), using the following code:
...
blob_client = BlobClient.from_connection_string(self.connection_string, "jobs", operation_id)
data_stream = await blob_client.download_blob()
raw_value = await data_stream.readall()
...

Using the same ab parameters (n=1000, c=50), we see a DRASTIC bottleneck showing, as the req/sec are reduced to around 20 - 25 rps:
Concurrency Level:      50
Time taken for tests:   43.861 seconds
Complete requests:      1000
Failed requests:        0
Non-2xx responses:      1000
Total transferred:      431000 bytes
HTML transferred:       126000 bytes
Requests per second:    22.80 [#/sec] (mean)
Time per request:       2193.062 [ms] (mean)
Time per request:       43.861 [ms] (mean, across all concurrent requests)
Transfer rate:          9.60 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        1    2   0.4      2       8
Processing:   193 2171 1116.8   1969    5131
Waiting:      193 2171 1116.8   1969    5130
Total:        194 2173 1116.8   1970    5133

Percentage of the requests served within a certain time (ms)
  50%   1970
  66%   2414
  75%   2964
  80%   3193
  90%   3767
  95%   4524
  98%   4788
  99%   5130
 100%   5133 (longest request)

This does not seem to match at all what the underlying storage account is reporting via metrics (max around 20ms, allowing for 50ms just to be safe). Additionally, total transactions is reporting nearly exactly the number of tests (~1000), indicated that general purpose storage account throttling is not occuring.
Given the very small size of these blobs, and the fact the underlying pipeline transport should be using the aiohttp provider, something feels like it is either overwhelming the event loop, or still using a blocking call unintentionally.
Would appreciate any insights that can be offered - while there will of course be overhead to calling storage (don't expect the same perf as the first test), there is seemingly quite a bit of un-accounted for time being taken somewhere. We definitely do not want to resort to implementing these calls via Tornado / etc. by hand if we don't have to.
Thanks!

UPDATE
Based on (this repo)[https://github.com/mikeharder/StoragePerf], re-ran tests that re-used a BlobClient. While this isn't applicable to our use case (each request will likely request a different blob), I wanted to share the results.
There was a positive change - up to around 50-60 rps - but nothing significant. Results from ab run are below:
Concurrency Level:      50
Time taken for tests:   17.459 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      365000 bytes
HTML transferred:       73000 bytes
Requests per second:    57.28 [#/sec] (mean)
Time per request:       872.964 [ms] (mean)
Time per request:       17.459 [ms] (mean, across all concurrent requests)
Transfer rate:          20.42 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        1    2   1.1      2      22
Processing:    44  858 482.3    771    2691
Waiting:       44  857 482.3    771    2690
Total:         45  860 482.4    772    2693

Percentage of the requests served within a certain time (ms)
  50%    772
  66%    992
  75%   1114
  80%   1217
  90%   1447
  95%   1715
  98%   2335
  99%   2489
 100%   2693 (longest request)

@mjyeaney thank you so much for reporting this. Our team will take a look at this //cc: @mayurid, @rakshith91

@mjyeaney  Thanks for reporting the issue.
Could you let me know what version of python are you using, so that i can reproduce this locally using the same version.

Local dev environment: Python 3.6.8, pip 9.0.1
Dockerfile: FROM python:3.6

@mjyeaney : I have a few follow up questions

Is your scenario is to download 1000 blobs from the same container or is it a different container everytime?
I would like to know if you are comparing the results to 2.x version of the storage-blobs?

Also, by creating a new blob client every time from a connection string, you are not using a "connection pool". So the ideal usage for optimal performance would be something like this:
container_client = ContainerClient.from_connection_string(connection_string, "jobs")
async with container_client:
    for operation_id in list_of_blobs:
        blob_client = container_client.get_blob_client(operation_id)
        data_stream = await blob_client.download_blob()
        raw_value = await data_stream.readall()

Also note the "async with" block. This is required to close the client session properly, or it would throw a warning something like:
Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x0447DCF0>

Finally, if you want to use more than one thread, you must pass max_concurrency param to the download_blob.

Here's the scenario:

High throughput job status retrieval (many concurrent requests, very high likelyhood that each is after a different blob).
1000 was picked just for a nice averaging load...however, in this very simple test everybody is asking status on the same blob (TODO: If this is a possible issue, I can modify the tests to all be different).

With regards to connection pooling per-client, didn't realize BlobClient worked that way (docs don't cover this behavior - this is very important indeed! TODO: Should this be a docs PR?).
Finally - we won't have a loop as shown above. Each incoming request will act independently, but let me re-work this into leveraging a container client first..again, wasn't sure what the guidance was as the docs don't dive into any specifics :-(
Will post updates here shortly.

Just changed our job store to the following (simplified):
class JobStore:
    def __init__(self, connection_string):
        self.connection_string = connection_string
        self.container_client = ContainerClient.from_connection_string(connection_string, "jobs")

    async def read_job(self, operation_id):
        blob_client = self.container_client.get_blob_client(operation_id)
        data_stream = await blob_client.download_blob()
        raw_value = await data_stream.readall()
        return json.loads(raw_value)

Results are below...no notable change even when re-using the container client to get a blob client.
Concurrency Level:      50
Time taken for tests:   39.199 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      365000 bytes
HTML transferred:       73000 bytes
Requests per second:    25.51 [#/sec] (mean)
Time per request:       1959.974 [ms] (mean)
Time per request:       39.199 [ms] (mean, across all concurrent requests)
Transfer rate:          9.09 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        1    2   0.8      2      10
Processing:    73 1929 1152.1   1755    5684
Waiting:       73 1929 1152.1   1755    5684
Total:         75 1931 1152.1   1757    5685

Percentage of the requests served within a certain time (ms)
  50%   1757
  66%   2078
  75%   2795
  80%   2913
  90%   3667
  95%   4265
  98%   4639
  99%   4711
 100%   5685 (longest request)

Also tried specifying max_threads=50 during the download_blob call, slightly increased calls up to 26.86, but variance in tests was making it hard to see if it was anything but noise.

Just changed our job store to the following (simplified):
class JobStore:
    def __init__(self, connection_string):
        self.connection_string = connection_string
        self.container_client = ContainerClient.from_connection_string(connection_string, "jobs")

    async def read_job(self, operation_id):
        blob_client = self.container_client.get_blob_client(operation_id)
        data_stream = await blob_client.download_blob()
        raw_value = await data_stream.readall()
        return json.loads(raw_value)

Results are below...no notable change even when re-using the container client to get a blob client.
Concurrency Level:      50
Time taken for tests:   39.199 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      365000 bytes
HTML transferred:       73000 bytes
Requests per second:    25.51 [#/sec] (mean)
Time per request:       1959.974 [ms] (mean)
Time per request:       39.199 [ms] (mean, across all concurrent requests)
Transfer rate:          9.09 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        1    2   0.8      2      10
Processing:    73 1929 1152.1   1755    5684
Waiting:       73 1929 1152.1   1755    5684
Total:         75 1931 1152.1   1757    5685

Percentage of the requests served within a certain time (ms)
  50%   1757
  66%   2078
  75%   2795
  80%   2913
  90%   3667
  95%   4265
  98%   4639
  99%   4711
 100%   5685 (longest request)

Also tried specifying max_threads=50 during the download_blob call, slightly increased calls up to 26.86, but variance in tests was making it hard to see if it was anything but noise.

Just for comparison, I coded up a manual storage REST API call (implemented in the repo linked above, in src/storage_rest_api.py. Results are below...leveraging aiohttp, consistently hitting ~117 rps from within the datacenter running ab with the same configuration of n=1000, c=50:
Concurrency Level:      50
Time taken for tests:   8.529 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      365000 bytes
HTML transferred:       73000 bytes
Requests per second:    117.25 [#/sec] (mean)
Time per request:       426.451 [ms] (mean)
Time per request:       8.529 [ms] (mean, across all concurrent requests)
Transfer rate:          41.79 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        1    2   0.6      2      10
Processing:    42  419 147.5    413     828
Waiting:       42  419 147.5    413     828
Total:         45  421 147.5    415     830

Percentage of the requests served within a certain time (ms)
  50%    415
  66%    481
  75%    527
  80%    556
  90%    622
  95%    666
  98%    724
  99%    775
 100%    830 (longest request)

This manual code is hardly optimized and hitting a little over 5x perf gains with far better behaved latency distribution. Seems to reinforce the theory of queueing/contention in the SDK use case...hopefully we can isolate (I really don't want to manage hand-writing calls like this LOL).

QUICK UPDATE
Made some optimizations here
https://github.com/Azure/azure-sdk-for-python/pull/8698
Will create one more PR for updating documentation and will continue inspecting performance with downloading same blob and play with other scenarios

@mjyeaney I have tried downloading the same blob too and surprisingly the performance has improved. I'll try to run the same script you are using instead of my own.
Can you point out how you are running the script from your repo

Sure thing - the way I was running the load test was the following:

Deployed the test app Docker image to the App Service
SSH to the load test machine (Ds2v3 box) and simply run:
ab -n 1000 -c 50 http://{app_svc_url}/path/to/simple/endpoint

That's it..not super scientific, but since these were all HTTP GET requests, it is super-easy to run over and over.
That said, I've been moving forward with using the SDK and am continuing to build out / deploy code, so simply cloning that repo likely won't give you the same endpoint.  As an easier test - if you have a beta drop of the SDK I can try that to see how it's working.
As for azure-core, here's what I have currently installed in my virtualenv:
$> pip list | grep azure-core
azure-core (1.0.0)

Is that the dependency you were talking about?

Is that the dependency you were talking about?

Right! If you want to check with the editable version of azure-core temporarily, that'd be great.
git clone git@github.com:Azure/azure-sdk-for-python.git
cd sdk/core/azure-core
pip install -e .

@mjyeaney We have just released the latest version of azure-core. Can you upgrade it using pip install azure-core==1.1.0 and see if you see improvements with results? Thanks.

Perfect! I'll test it tomorrow and post results here.
Thanks!

Hi @mjyeaney
Did you get a chance to try this?

Yes @rakshith91 (and apologies for the delay!)
The results with the new azure-core 1.1.0 lib are quite positive - I'm seeing around a ~5x improvement in the initial use case of requesting a blob. Results from ab are below - note this is now yielding a repeatable ~95 - 100 req's/sec (where previously we were only getting ~22):
Concurrency Level:      30
Time taken for tests:   10.623 seconds
Complete requests:      1000
Failed requests:        0
Total transferred:      397000 bytes
HTML transferred:       99000 bytes
Requests per second:    94.13 [#/sec] (mean)
Time per request:       318.704 [ms] (mean)
Time per request:       10.623 [ms] (mean, across all concurrent requests)
Transfer rate:          36.49 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        1    2   0.6      1       8
Processing:    53  314 205.0    269    1116
Waiting:       53  314 205.0    269    1116
Total:         54  316 205.0    271    1118
WARNING: The median and mean for the initial connection time are not within a normal deviation
        These results are probably not that reliable.

Percentage of the requests served within a certain time (ms)
  50%    271
  66%    358
  75%    419
  80%    461
  90%    593
  95%    714
  98%    923
  99%   1020
 100%   1118 (longest request)

Also - note this is still on the same low-end dev/test hardware SKU (B2), so moving to production will only help this.
In all, definitely an improvement - easily approaching the results from a raw hand-coding against the REST API (as reported above). Definitely appreciate the improvements!
I'm hoping this can help guide further improvements in the SDK. Is there any additional information you need from these tests?

@mjyeaney No need to apologize :)
Thanks a lot for updating the results and glad to see the improvement. Will be closing the issue for now, but feel free to keep in touch if you think there may be more overheads.


[Feature] lcd rotation

It replace #37.
This PR adds support for the feeder to switch between multiple LCD clients in round-robin fashion in case one becomes unavailable. This becomes especially useful in the context of multi-party computation validator setups (see horcrux for Cosmos SDK-based blockchains).

Once the feeder notices an LCD client being unavailable, it just talks to the next one on its list, giving the operator enough time for troubleshooting of the failed one.

This is great, what is the syntax to specify multiple LCDs in order of preference?
How long does it take for it to consider that the current LCD is failed to switch to the next one?
How long does it take for it to check whether the previous one is back online, and switch back to it?
thx!

You just add as many --lcd flags as you like (see the feeder README). The order of the addresses determines the rotation order.
As soon as one LCD client becomes unavailable, the feeder tries to talk to the next one until it finds an available one.
It doesn't check whether any previous LCDs are back online. As long as the current LCD is available, it will not switch to another one.

ok, one last q: under which condition does it consider an LCD unavailable?  only when it doesn't reply? what if it is sluggish, or replies with a block in the past?
PS: I'm prob asking for too much here, I know ;)


gollum: 5.3.3 -> 6.0.1

Description of changes
Changelog
Migration notes

Things done

Built on platform(s)

[X] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)

[ ] sandbox = relaxed
[ ] sandbox = true

[ ] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[ ] Fits CONTRIBUTING.md.

Add a :+1: reaction to pull requests you find important.

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
Result of nixpkgs-review pr 330840 run on x86_64-linux 1

2 packages blacklisted:

nixos-install-tools
tests.nixos-functions.nixos-test

2 packages built:

gollum
nixpkgs-manual

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

nixpkgs-manual:

Package is missing maintainers.
If the package is using runCommand please make sure to inherit or list one or more maintainers.


Feature: Update 0.11 - Config Revolution

Changes include:

Improved settings display
Added settings handling to display settings previews according to the config object
Added setting for playlist alignment mode

README.md conflict is the result of overwriting changes in both branches at 0.10.10, resolved


Attempt to add 'case statements' to indexing

This is just for a search convenience if someone
coming from a C-style language background searches for 'case' or
'case statements' instead of 'switch'.
The problem
As part of #3393
If the user exploring Raku has not encountered given/when in Raku before,
they might search the site using 'case' or 'case statement'.  Right now those terms
do not take the user directly to the given/when documentation.
Solution provided
I am not skilled with pod6 format, so it is likely I got the syntax wrong.  But
I tried to make a change that would add 'case statements' to the search index.

Thanks again for the review and feedback.  Is this update what you had in mind?


Fixes double counting when adjusting third chart

Added mono attribute to tri-colors
Adjusted calculations to not count monos twice.

Surged!


3.Sprinting - Start coding phase Services

Coding review:

Positive aspects

laconic
consistent
use separate DB table for services content
no redundant code
no errors
work as designed
no flaws or undescribed functionality
use all service parameters from the table
meaningful and logic table columns nomination
simple and efficient back-end code block

Aspects to improve

verify with the e-commerce owner necessity of additional services


npm install error at latest version on Windows 10

node: v16.15.0 --LTS
yarn: 1.22.18
Windows:  Windows 10, version 21H2, 19044.1706

mkdir install_minidump_demo
yarn init -y
yarn add minidump

yarn add v1.22.18
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
error C:\Users\bingzhang\devspace\install_minidump_demo\node_modules\minidump: Command failed.
Exit code: 1
Command: node build.js
Arguments:
Directory: C:\Users\bingzhang\devspace\install_minidump_demo\node_modules\minidump
Output:
C:\Users\bingzhang\devspace\install_minidump_demo\node_modules\minidump\build.js:24
  if (result.error) throw result.error
                    ^

Error: spawnSync C:\Users\bingzhang\devspace\install_minidump_demo\node_modules\minidump\deps\breakpad\configure ENOENT
    at Object.spawnSync (node:internal/child_process:1111:20)
    at Object.spawnSync (node:child_process:814:24)
    at spawnSync (C:\Users\bingzhang\devspace\install_minidump_demo\node_modules\minidump\build.js:23:31)
    at Object.<anonymous> (C:\Users\bingzhang\devspace\install_minidump_demo\node_modules\minidump\build.js:42:1)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at node:internal/main/run_main_module:17:47 {
  errno: -4058,
  code: 'ENOENT',
  syscall: 'spawnSync C:\\Users\\bingzhang\\devspace\\install_minidump_demo\\node_modules\\minidump\\deps\\breakpad\\configure',
  path: 'C:\\Users\\bingzhang\\devspace\\install_minidump_demo\\node_modules\\minidump\\deps\\breakpad\\configure',
  spawnargs: []
}
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.

The file node_modules\minidump\deps\breakpad\configure is exist:

I meet the same problem when i use node 16.0.0 on windows 10

I meet the same problem on Windows 11

not support win32


add websocket listener config (plain and ssl)

defaults to same config as the original (plain mqtt enabled, all other disabled) but adds ability to configure websockets listeners for both plain and ssl

Hi @ToSa27,
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!

@pvizeli done - please check again.
I would have celled it 1.1 because there are no breaking changes, just an added feature which should qualify for a minor increase not a major but looking at other addons it seems like you don't make that distinction in this repo, hence I did the requested change to 2.0

Please update the documentation on website too

Fix: #338


Remove complex amp support for ScalableSymbolicPulse

Summary
Support for complex amp in the instantiation of ScalableSymbolicPulse is removed.
Details and comments
Complex amp support was deprecated in #10357 (with some parts awaiting deprecation with #11257 in the 0.46 branch). This PR removes the support.
Pulses with complex amp loaded from old QPY files are automatically converted to (amp,angle) representation (with a user warning). Note that there are three eras with regard to this - complex amp (Terra <0.23), amp,angle with complex amp support (Qiskit < 1.0) and now amp,angle with no complex amp support (Qiskit >= 1.0). QPY version was not bumped, so the "complexity" of amp is tested even for QPY files of Qiskit >=1.0. With the next version of QPY, amp could be assumed to be real.
The QPY compatibility test was updated to reflect the difference in the API, but backwards compatibility is maintained.

Thanks @nkanazawa1989 !
I'll take a look at the parameter assignment. I hope I can squeeze it in before the deadlines.


Patch 1

Hello,
I made a small modification to the calculation of the locationed memory in this model. Now it uses the absolute distance context word - aspect as specified in the paper.
Hope you find this useful!

@all-contributors please add @AlbertoPaz for code


How to make pick up / put down translation

Hi,
I trying implement  pick up / put down translation on Y axis in example hellosceneform. Now I set  setWorldPosition with modified Y-axis value when model is anchored, this picks up model.
How  to make this translation based on  gesture, like translation on Z or X axis. It is possible with TransformableNode / TranslationController ?
Vector3 cameraPos = this.sceneformFragment.getArSceneView().getScene().getCamera().getWorldPosition();
Vector3 cameraForward = this.sceneformFragment.getArSceneView().getScene().getCamera().getForward();
Vector3 cameraPosUp = new Vector3(cameraPos.x,0.35f,cameraPos.z);
Vector3 finalPosition = Vector3.add(cameraPosUp, cameraForward.scaled(1.0f));
transformableNode.setWorldPosition(finalPosition);

Thanks

If I understand the question correctly you are trying to add a completely new gesture that works with the existing ones, but moves an object up/down instead of on the xz plane?
If so I'll look into this.

@malik-at-work  Yes, you understand the question correctly.

I've created a working example for how to create an additional gesture and additional transformation controller for raising and lowering a node that can be checked out here.
It uses a two-finger drag gesture to raise/lower the node.
In general, these were the steps:

Create new subclasses of BaseGesture and BaseGestureRecognizer for TwoFingerDragGesture.
Create a new subclass of BaseTransformationController that uses the TwoFingerDragGesture to raise and lower the object.
Create a subclass of TransformableNode to integrate it all together.

One important note: The RaiseController works by raising/lowering a child of the TransformableNode, not the TransformableNode itself. This is so the RaiseController does not conflict with the TranslationController. To make this work, the Renderable is attached to the child of TransformableNode, not the TransformableNode itself.

@dsternfeld7 Thank you, I'll try it.


Add support for multiple files on deploy_dir option

LGTM. Will integrate this weekend

💃  let's do this 😄

@aitherios Now :)

@aitherios tsuru plugin-install bluegreen https://raw.githubusercontent.com/emerleite/tsuru-bluegreen/1.3.0/src/bluegreen.py


bugfix: fix readonly branch commit errors in Oracle XA transactions.

[ ] I have registered the PR changes.

Ⅰ. Describe what this PR did
fix the issue of readonly branch commit errors in Oracle XA transactions, which leads to long-running transactions being unable to commit and causing unreadable states.
Solution Logic:
When the database is Oracle and the prepare result is XA_RDONLY, notify TC that the current branch status is BranchStatus.PhaseOne_RDONLY (a newly added state to address this issue). During global commit, for branches with this status type, delete the branch log directly and ignore it.
Ⅱ. Does this pull request fix one issue?

fixes #6512
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Based on the problematic local project instance, the verification includes:
Whether the Seata TC frontend displays normally;
In Seata TM managing multiple RMs, whether it functions correctly when there is one or more readonly branches; whether it functions correctly when there are no readonly branches; and whether it functions correctly when there are only one or more readonly branches.
Ⅴ. Special notes for reviews

我们是否可以将sql进行解析，如果这个sql是select类型的sql，就不进行xa相关的事务动作？ Can we parse the SQL statement to skip XA-related transaction actions if the SQL is of type SELECT?

Simple branch logic composed of basic SQL can be implemented this way, but consideration is needed if the transaction involves more complex scenarios such as calling stored procedures under XA branch transactions. The business logic within stored procedures may not be straightforward to determine.
Moreover, in Oracle, the prepare phase can explicitly indicate whether the branch is read-only, so it may feel unnecessary to handle it in this way.

我们是否可以将sql进行解析，如果这个sql是select类型的sql，就不进行xa相关的事务动作？ Can we parse the SQL statement to skip XA-related transaction actions if the SQL is of type SELECT?

Simple branch logic composed of basic SQL can be implemented this way, but consideration is needed if the transaction involves more complex scenarios such as calling stored procedures under XA branch transactions. The business logic within stored procedures may not be straightforward to determine.
Moreover, in Oracle, the prepare phase can explicitly indicate whether the branch is read-only, so it may feel unnecessary to handle it in this way.

We currently refuse to use stored procedures directly, so I would prefer to place the XA START after the first DML action is recognized.

我们是否可以将sql进行解析，如果这个sql是select类型的sql，就不进行xa相关的事务动作？ Can we parse the SQL statement to skip XA-related transaction actions if the SQL is of type SELECT?

Simple branch logic composed of basic SQL can be implemented this way, but consideration is needed if the transaction involves more complex scenarios such as calling stored procedures under XA branch transactions. The business logic within stored procedures may not be straightforward to determine.
Moreover, in Oracle, the prepare phase can explicitly indicate whether the branch is read-only, so it may feel unnecessary to handle it in this way.

We currently refuse to use stored procedures directly, so I would prefer to place the XA START after the first DML action is recognized.

The official documentation mentions for XA use cases: Suitable for migrating old applications to the Seata platform based on the XA protocol.. Therefore, if stored procedures are not supported, it will not be possible to achieve this. Currently, some of our applications are using a mix of XA and TCC.
If the XA transaction is started only after the first DML operation, in Oracle, this can be achieved by leveraging the Promotable XA feature to ensure that the repeatable read characteristic is maintained under the Repeatable Read isolation level. Otherwise, the repeatable read cannot be guaranteed. This feature is not necessarily supported in other databases.

I don't understand the relevance between supporting stored procedures and migrating to Seata XA mode that you mentioned.
I believe Oracle commonly uses the RC isolation level, so it seems there's no need to use the feature you mentioned.

I don't understand the relevance between supporting stored procedures and migrating to Seata XA mode that you mentioned.
I believe Oracle commonly uses the RC isolation level, so it seems there's no need to use the feature you mentioned.

Currently, we have some legacy services that process business logic through stored procedures and we want to integrate them with Seata transactions.
We are using the RR (Repeatable Read) isolation level, and since Seata is a general framework, XA as a two-phase commit protocol should support various scenarios.
The basic logical template structure of XA transactions is consistent, with only minor implementation differences across different databases. However, all of them can handle the basic logical template well. For the current issue, the desired result can be achieved during the prepare phase, and appropriate processing can be performed. There is no need to introduce other processing logic outside of the XA process.

The current issue of read-only transaction branch commit errors can be efficiently and elegantly resolved within the internal handling logic of XA transactions.
Since SQL parsing cannot adapt to all database syntaxes, introducing SQL parsing to solve this issue would undoubtedly reduce the generality of XA transactions and increase performance overhead due to SQL parsing.

Based on the above points, it is not recommended to introduce SQL parsing to address this issue.

I understand your point, but I believe that in a read-only transaction, why isn't it considered a local transaction instead of being included in a global transaction? If a global transaction includes a read-only local transaction, it will inevitably affect throughput due to this additional branch. However, if it is just a local transaction, there will be significantly fewer interactions with the DB and the TC. Therefore, I believe that the overhead caused by SQL parsing is much lower than the overhead of starting an XA transaction and registering an XA branch to the TC. Moreover, we only need to identify whether the SQL is a SELECT statement rather than other DML statements. This has little to do with the number or variety of SQL types supported by SQL parsing.

I understand your point, but I believe that in a read-only transaction, why isn't it considered a local transaction instead of being included in a global transaction? If a global transaction includes a read-only local transaction, it will inevitably affect throughput due to this additional branch. However, if it is just a local transaction, there will be significantly fewer interactions with the DB and the TC. Therefore, I believe that the overhead caused by SQL parsing is much lower than the overhead of starting an XA transaction and registering an XA branch to the TC. Moreover, we only need to identify whether the SQL is a SELECT statement rather than other DML statements. This has little to do with the number or variety of SQL types supported by SQL parsing.

The branch transactions in this scenario are typically executed based on the business state, which determines whether data insertions, deletions, or updates are necessary. Different business data states lead to inconsistencies in this processing logic—sometimes it may be read-only, sometimes not.
While it can be seen as an imperfect organization and implementation of business logic, due to resource constraints, it is often difficult to handle every detail thoroughly.
The solution I am currently proposing addresses this issue by adding an additional interaction with the Transaction Coordinator (TC) when such a situation arises, to avoid errors in the commit of read-only branch transactions. This can be understood as a fault tolerance mechanism. Of course, in a well-organized and clear business logic process, as you mentioned, such situations would not occur, and thus this extra interaction with the TC would not be necessary.

Of course, the proposed solution can later be combined with the SQL parsing logic you mentioned. If the SQL parsing perfectly handles all operations that are read-only and avoids initiating XA-related transaction actions, it would also prevent this extra interaction with the TC. However, if there are any omissions, this approach can improve robustness.
Furthermore, this proposed solution can effectively and immediately resolve the current issue with Oracle XA branch transactions. It can prevent the problem where, after the global transaction is committed, other XA branches' data remain unreadable for an extended period due to the failure of committing read-only transaction branches.

Regarding read-only branches in relation to SQL parsing recognition, there are additional actions such as XA end and XA prepare, all of which involve I/O interactions with the database. Then there's the step of reporting the branch status to the transaction coordinator (TC), which adds some extra overhead.

I'm not quite clear on what you're saying. Why would a failed commit of an XA read-only branch affect other transactions' ability to read data? It shouldn't have acquired any data locks; a snapshot read shouldn't hold any exclusive database locks.

I understand the intention behind your pull request, which indeed optimizes behavior under Oracle's XA read-only transactions. However, I would prefer a more universal approach applicable to all compatible databases, aiming for standardized handling. Therefore, I propose that after SQL parsing, we should skip initiating XA transaction-related actions for non-DML operations. We might need input from other community members on this point. @slievrly @PeppaO

Regarding read-only branches in relation to SQL parsing recognition, there are additional actions such as XA end and XA prepare, all of which involve I/O interactions with the database. Then there's the step of reporting the branch status to the transaction coordinator (TC), which adds some extra overhead.
I'm not quite clear on what you're saying. Why would a failed commit of an XA read-only branch affect other transactions' ability to read data? It shouldn't have acquired any data locks; a snapshot read shouldn't hold any exclusive database locks.
I understand the intention behind your pull request, which indeed optimizes behavior under Oracle's XA read-only transactions. However, I would prefer a more universal approach applicable to all compatible databases, aiming for standardized handling. Therefore, I propose that after SQL parsing, we should skip initiating XA transaction-related actions for non-DML operations. We might need input from other community members on this point. @slievrly @PeppaO

The issue in point 2 is that when an XA transaction commits globally, the multiple branch transactions in the TC are committed synchronously, one after the other. If any of these commits fail, the commit logic exits, causing the global commit to get stuck on that failed branch, continuously retrying. This prevents subsequent RM branches from actually receiving the TC's commit request, making the transactions of these RM branches invisible to other transactions.

Regarding read-only branches in relation to SQL parsing recognition, there are additional actions such as XA end and XA prepare, all of which involve I/O interactions with the database. Then there's the step of reporting the branch status to the transaction coordinator (TC), which adds some extra overhead.
I'm not quite clear on what you're saying. Why would a failed commit of an XA read-only branch affect other transactions' ability to read data? It shouldn't have acquired any data locks; a snapshot read shouldn't hold any exclusive database locks.
I understand the intention behind your pull request, which indeed optimizes behavior under Oracle's XA read-only transactions. However, I would prefer a more universal approach applicable to all compatible databases, aiming for standardized handling. Therefore, I propose that after SQL parsing, we should skip initiating XA transaction-related actions for non-DML operations. We might need input from other community members on this point. @slievrly @PeppaO

The issue in point 2 is that when an XA transaction commits globally, the multiple branch transactions in the TC are committed synchronously, one after the other. If any of these commits fail, the commit logic exits, causing the global commit to get stuck on that failed branch, continuously retrying. This prevents subsequent RM branches from actually receiving the TC's commit request, making the transactions of these RM branches invisible to other transactions.

I understand. So that's why you've implemented the skip operation for this branch. I see this PR as a temporary solution, but ultimately, we should move towards a unified processing model

Of course, the proposed solution can later be combined with the SQL parsing logic you mentioned. If the SQL parsing perfectly handles all operations that are read-only and avoids initiating XA-related transaction actions, it would also prevent this extra interaction with the TC. However, if there are any omissions, this approach can improve robustness.

I understand. So that's why you've implemented the skip operation for this branch. I see this PR as a temporary solution, but ultimately, we should move towards a unified processing model

Yes, this PR can be seen as a temporary solution, or as mentioned above, it can serve as a fallback solution to ensure robustness.

One of the reasons for using XA transactions in Seata is that XA transactions allow developers to:

Fully utilize the database's unique features: For example, special SQL syntax, stored procedures, functions, etc.
Standardized interface: XA transactions are implemented based on a standard protocol with a first phase (start, execute, end, prepare) and a second phase (commit/rollback).

I believe that the implementation of XA transactions in Seata should maintain these basic characteristics of XA transactions. From this perspective, the approach in this PR:

Does not compromise the support for the database's unique features.
Does not exceed the requirements of the XA standard protocol.
Indeed solves the issue of read-only branch commit errors.

Therefore, I hope this PR can be accepted.

@funky-eyes fix the incorrect code style: '{' is not preceded with whitespace.

@funky-eyes fix the incorrect code style: '{' is not preceded with whitespace.

OK, thank you for submitting this PR. However, due to the discussions mentioned above, I need to consult with other committers and PPMC members in the community. Please be patient while I do so.

@funky-eyes I tested four types of databases: MySQL (8), Oracle (12c), Postgres (16), and MSSQL Server (2022). Only Oracle has read-only optimization; the others do not provide read-only feedback. Based on these four databases, the database type check can be eliminated here.

This is awesome, but why are you closing this PR?

in

This is awesome, but why are you closing this PR?

@funky-eyes Misoperation


fix(topn-states): top-n do not have parallelism so use operator id

What's changed and what's your intention?
PLEASE DO NOT LEAVE THIS EMPTY !!!
Please explain IN DETAIL what the changes are in this PR and why they are needed:

Summarize your change (mandatory)
How does this PR work? Need a brief introduction for the changed logic (optional)
Describe clearly one logical change and avoid lazy messages (optional)
Describe any limitations of the current code (optional)

Checklist

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

Refer to a related PR or issue link (optional)

Whether to use executor_id and operator_id depends on "If we have parallelism, can the executor share state." TopN cannot share the same keyspace indeed, so I think it's better leave this unchanged in case someday we accidentally add the parallelism, or have something like two-phase topN.

Good. Let's hold it unchanged.


Make SelectionManager more resilient

Fixes #809

Coverage decreased (-0.2%) to 70.377% when pulling e56f710cd1ce2e4aa15ea8af8bd5b0cc8cfd736e on Tyriar:809_onSingleClick_null_check into 18f3cc77e51d54ce06f37d6fde42d2e762cabcb3 on sourcelair:master.

Something we have done in JupyterLab is temporarily turn on strictNullChecks and fix as many local errors as we can, and then turn the flag off.  We have a couple of type system restrictions that prevent us from turning it on permanently.  One annoying thing is that getComputedStyle attributes are all possibly null, leading to style.width! (notice the null assertion operator).

It would be good to be as strict as possible eventually, can't wait until https://github.com/sourcelair/xterm.js/issues/335 is done

Coverage decreased (-0.2%) to 70.396% when pulling bbb5daa5753584a9715ca7fd53e8d35c656f0ac7 on Tyriar:809_onSingleClick_null_check into 739bd05037a1678d0550934a0bb9eec6bfdbeb81 on sourcelair:master.

Coverage decreased (-0.2%) to 70.396% when pulling bbb5daa5753584a9715ca7fd53e8d35c656f0ac7 on Tyriar:809_onSingleClick_null_check into 739bd05037a1678d0550934a0bb9eec6bfdbeb81 on sourcelair:master.


fix: incorrect prompt type breaking gptme benchmarks

The gptme benchmarking suite uses strings for prompts which causes an error because agent#improve assumes it's a Prompt object.

Looks good to me! @ErikBjare, can I ask you to take a look and see if this is fine?


tf.function(jit_compile=True) cannot access int32 weights on GPU

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

Have I written custom code (as opposed to using a stock example script provided in TensorFlow):
yes
OS Platform and Distribution (e.g., Linux Ubuntu 16.04):
Ubuntu 20.04.3 LTS
Mobile device (e.g. iPhone 8, Pixel 2, Samsung Galaxy) if the issue happens on mobile device:
TensorFlow installed from (source or binary):
TensorFlow version (use command below):
v2.6.0-0-g919f693420e 2.6.0
Python version:
Python 3.8.10
Bazel version (if compiling from source):
GCC/Compiler version (if compiling from source):
CUDA/cuDNN version:
11.3 8.2
GPU model and memory:
A100 40GB

Describe the current behavior
Functions with tf.function(jit_compile=True) cannot access weights of type int32 on the GPU.  It gives error at runtime
2021-11-24 15:12:28.148837: W tensorflow/core/framework/op_kernel.cc:1692] OP_REQUIRES failed at xla_ops.cc:238 : Invalid argument: Trying to access resource _AnonymousVar2 (defined @ /lus/theta-fs0/software/thetagpu/conda/2021-09-22/mconda3/lib/python3.8/site-packages/keras/engine/base_layer_utils.py:117) located in device /job:localhost/replica:0/task:0/device:CPU:0 from device /job:localhost/replica:0/task:0/device:GPU:0

Describe the expected behavior
no error
Contributing

Do you want to contribute a PR? (yes/no):
no
Briefly describe your candidate solution(if contributing):
no solution

Standalone code to reproduce the issue
Provide a reproducible test case that is the bare minimum necessary to generate
the problem. If possible, please share a link to Colab/Jupyter/any notebook.
Please just run the following code,
import tensorflow as tf
import tensorflow.keras as tk

class Scale(tk.layers.Layer):
    def __init__(self, c, dt):
        super(Scale, self).__init__(name='Scale')
        self.c = self.add_weight(initializer=tf.keras.initializers.Constant(c), dtype=dt)
    @tf.function(jit_compile=True)
    def call(self, x):
        return self.c*x

print('float32:', Scale(2, tf.float32)(tf.ones(4, dtype=tf.float32)))
print('int64:', Scale(2, tf.int64)(tf.ones(4, dtype=tf.int64)))
print('int32:', Scale(2, tf.int32)(tf.ones(4, dtype=tf.int32)))  # This line gives error

Other info / logs Include any logs or source code that would be helpful to
diagnose the problem. If including tracebacks, please include the full
traceback. Large logs and files should be attached.
Below is the output from running the above code,
$ python issue_int32.py
2021-11-24 15:16:53.015406: I tensorflow/core/platform/cpu_feature_guard.cc:142] This TensorFlow binary is optimized with oneAPI Deep Neural Network Library (oneDNN) to use the following CPU instructions in performance-critical operations:  SSE3 SSE4.1 SSE4.2 AVX AVX2 FMA
To enable them in other operations, rebuild TensorFlow with the appropriate compiler flags.
2021-11-24 15:16:53.423483: I tensorflow/core/common_runtime/gpu/gpu_device.cc:1510] Created device /job:localhost/replica:0/task:0/device:GPU:0 with 38458 MB memory:  -> device: 0, name: NVIDIA A100-SXM4-40GB, pci bus id: 0000:bd:00.0, compute capability: 8.0
2021-11-24 15:16:53.776487: I tensorflow/compiler/xla/service/service.cc:171] XLA service 0x562deb0ee730 initialized for platform CUDA (this does not guarantee that XLA will be used). Devices:
2021-11-24 15:16:53.776535: I tensorflow/compiler/xla/service/service.cc:179]   StreamExecutor device (0): NVIDIA A100-SXM4-40GB, Compute Capability 8.0
2021-11-24 15:16:53.818100: I tensorflow/compiler/jit/xla_compilation_cache.cc:363] Compiled cluster using XLA!  This line is logged at most once for the lifetime of the process.
float32: tf.Tensor([2. 2. 2. 2.], shape=(4,), dtype=float32)
int64: tf.Tensor([2 2 2 2], shape=(4,), dtype=int64)
2021-11-24 15:16:53.865986: W tensorflow/core/framework/op_kernel.cc:1692] OP_REQUIRES failed at xla_ops.cc:238 : Invalid argument: Trying to access resource _AnonymousVar2 (defined @ /lus/theta-fs0/software/thetagpu/conda/2021-09-22/mconda3/lib/python3.8/site-packages/keras/engine/base_layer_utils.py:117) located in device /job:localhost/replica:0/task:0/device:CPU:0 from device /job:localhost/replica:0/task:0/device:GPU:0
Traceback (most recent call last):
  File "issue_int32.py", line 14, in <module>
    print('int32:', Scale(2, tf.int32)(tf.ones(4, dtype=tf.int32)))
  File "/lus/theta-fs0/software/thetagpu/conda/2021-09-22/mconda3/lib/python3.8/site-packages/keras/engine/base_layer.py", line 1037, in __call__
    outputs = call_fn(inputs, *args, **kwargs)
  File "/lus/theta-fs0/software/thetagpu/conda/2021-09-22/mconda3/lib/python3.8/site-packages/tensorflow/python/eager/def_function.py", line 885, in __call__
    result = self._call(*args, **kwds)
  File "/lus/theta-fs0/software/thetagpu/conda/2021-09-22/mconda3/lib/python3.8/site-packages/tensorflow/python/eager/def_function.py", line 956, in _call
    return self._concrete_stateful_fn._call_flat(
  File "/lus/theta-fs0/software/thetagpu/conda/2021-09-22/mconda3/lib/python3.8/site-packages/tensorflow/python/eager/function.py", line 1963, in _call_flat
    return self._build_call_outputs(self._inference_function.call(
  File "/lus/theta-fs0/software/thetagpu/conda/2021-09-22/mconda3/lib/python3.8/site-packages/tensorflow/python/eager/function.py", line 591, in call
    outputs = execute.execute(
  File "/lus/theta-fs0/software/thetagpu/conda/2021-09-22/mconda3/lib/python3.8/site-packages/tensorflow/python/eager/execute.py", line 59, in quick_execute
    tensors = pywrap_tfe.TFE_Py_Execute(ctx._handle, device_name, op_name,
tensorflow.python.framework.errors_impl.InvalidArgumentError: Trying to access resource _AnonymousVar2 (defined @ /lus/theta-fs0/software/thetagpu/conda/2021-09-22/mconda3/lib/python3.8/site-packages/keras/engine/base_layer_utils.py:117) located in device /job:localhost/replica:0/task:0/device:CPU:0 from device /job:localhost/replica:0/task:0/device:GPU:0 [Op:__inference_call_44]

@sachinprasadhs ,
I was able to reproduce the issue in tf v2.7, v2.5 and nightly.Please find the gist.

There is a known issue with tf.Variable: https://www.tensorflow.org/xla/known_issues#tfvariable_on_a_different_device
The self.v here is an int32 tf.Variable that can't be placed on GPU. You are suggested to use int64 or int32 + jit_compile=False.

Assuming the developers have a good reason for this odd behavior of int32, it would be useful for users to make this behavior controllable.  Is there a way to force int32 tensors on a GPU?
At the very least the error message could be much more useful that what it is now.  Complaining about an _AnonymousVarNNN located on a different device that where it is supposed to be is useless.

Assuming the developers have a good reason for this odd behavior of int32

There is no good reason to do that. It behaves that way because TF lacks a dedicated type for shapes, which must reside on CPU. We're looking into ways to fix this, but it's complicated and will take a while.
+1  to improving the error message.

@jxy @mdanatg From what I see the real bug is that device assignment is ignored and the variable is placed on CPU despite explicit GPU placement (speaking of shapes, do those ever reside inside variables though?). Once the placement on CPU happens, there's nothing much we can do: what would you expect to happen? Extra H2D copy potentially ruining performance? We can't really pattern match this case to a specific error message, as that would just create a more fragile system with more levels of indirection swallowing errors.

Just making it clearer that the variable (and which variable) was placed on CPU would help. The information is there (e.g. we have the internal name, and line number), so surfacing more clearly would help (e.g. include the actual line of code). "_AnonymousVarNNN" and "__inference_call_44" don't say much.
Agree that there's otherwise not much to do by the time things got here. Might be worth trying tf.config.set_soft_device_placement(False), though that has caveats too.

Does leaving random comments count as activity?

@jxy I'm not sure to be honest. Filing a new bug about improved error message would be probably better, what would you like to see? Just a better stack trace presentation, or TF somehow pattern-matching the error message into "ah you are using int32 weights inside a compiled function run on GPU, why not make them int64"? Or would you prefer TF to automatically copy the weights? (CC @smit-hinsu )


RetryAnalyzer doesn't work with DataProvider (error: wrong number of arguments)

TestNG Version

Note: only the latest version is supported

Version: 7.4.0
<dependency>
    <groupId>org.testng</groupId>
    <artifactId>testng</artifactId>
    <version>7.4.0</version>
</dependency>

Expected behavior
Rerun tests
Actual behavior
Failing to  wrong number of arguments
Is the issue reproducible on runner?

[ ] Shell
[X] Maven
[ ] Gradle
[ ] Ant
[ ] Eclipse
[X] IntelliJ
[ ] NetBeans

Issue
When have DataProvider and for test params set Array of Objects it's working fine, but when add Retry.class it's failing due wrong number of arguments, however when unpackage that Array and in test set exact Objects it's working fine with Retry.class
Test case sample
Failing on Rerun due  (wrong number of arguments)
   @DataProvider(name = "dataProvider", parallel = true)
    public Object[][] getTestData() throws Exception {
        return new Integer[][]{
                new Integer[]{5, 1, 15}};
    }

    @Test( dataProvider = "dataProvider", description = "set warning message",retryAnalyzer = Retry.class)
    public void simpleTest(Integer [] integers) throws Exception {
        Assert.assertFalse(true);
    }

Working fine without Retry
   @DataProvider(name = "dataProvider", parallel = true)
    public Object[][] getTestData() throws Exception {
        return new Integer[][]{
                new Integer[]{5, 1, 15}};
    }

    @Test( dataProvider = "dataProvider", description = "set warning message")
    public void simpleTest(Integer [] integers) throws Exception {
        Assert.assertFalse(true);
    }

Working fine with Retry
    @DataProvider(name = "dataProvider", parallel = true)
    public Object[][] getTestData() throws Exception {
        return new Integer[][]{
                new Integer[]{5, 1, 15}};
    }

    @Test( dataProvider = "dataProvider", description = "set warning message",retryAnalyzer = Retry.class)
    public void simpleTest(Integer one, Integer two, Integer three) throws Exception {
        Assert.assertFalse(true);
    }

Could you please share the code in Retry class?

@maluyvova - This is no longer a problem with the current code base and the fix should be available for you, once TestNG 7.5.0 has been released
You can work with the snapshot and check this for yourself
https://oss.sonatype.org/content/repositories/snapshots/org/testng/testng/7.5-SNAPSHOT/

@juherr - I noticed that we are not using the x.y.z versioning strategy after the recent refactoring.
Any idea how to go about fixing this ?
I noticed that the lines https://github.com/cbeust/testng/blob/a397e1b79e5ccb6c5676ab4b1f1311484c93fd71/build.gradle.kts#L6-L7
are adding the version as 7.5-SNAPSHOT instead of it being 7.5.0-SNAPSHOT. I am not sure how we can go about fixing this though.

@krmahadevan I will check next week, after my hollidays.

@krmahadevan Is it important? You can add the .0 here: https://github.com/cbeust/testng/blob/master/gradle.properties#L8

@juherr - the x.y.z numbering would be important if we are going to be doing patch releases as well Like for e.g., we can right now consider doing a 7.4.1. I will make a change in this file to fix this.

Ok. I think x.y.0 and x.y are similar: https://blog.soebes.de/blog/2017/02/04/apache-maven-how-version-comparison-works/
BTW, there is no problem to add a trailing 0 :)


Initial polling for motions in client

Merged into the development branch, even if it's WIP. The client can be finished with other PRs, becuase all changes in one PR would be too much.


Azure OpenAI client transcribes or translates speech depending on the deployment used, not the library function

Confirm this is an issue with the Python library and not an underlying OpenAI API

[X] This is an issue with the Python library

Describe the bug
The behaviour of the AzureOpenAI client depends on the deployment used and not on the library function called.
For an OpenAI client client, client.audio.transcriptions transcribes speech and client.audio.translations translates it, as expected.
For an AzureOpenAI client azure_client, however, you get a transcription or a translation depending on the endpoint specified regardless of the library function used. The library function used only determines the class of the result object, not the text.
To Reproduce

Observe OpenAI client behavior.

import os
from openai import OpenAI, AzureOpenAI
from azure.identity import get_bearer_token_provider, DefaultAzureCredential

# Example file: https://upload.wikimedia.org/wikipedia/commons/b/b1/Candide_01_voltaire.mp3
audio_file = "./Candide_01_voltaire.mp3"

openai_client = OpenAI()

with open(audio_file, 'rb') as f:
    result = openai_client.audio.transcriptions.create(
        file=f,
        model="whisper-1",
    )

    print(f"openai_client.audio.transcriptions: {result.__class__.__name__, result.text[:60]}")

    result = openai_client.audio.translations.create(
        file=f,
        model="whisper-1",
    )

    print(f"openai_client.audio.translations: {result.__class__.__name__, result.text[:60]}")

openai_client.audio.transcriptions: ('Transcription', "Chapitre premier de Candide ou l'optimisme, de Voltaire, enregistré pour LibriVo")
openai_client.audio.translations: ('Translation', 'Chapter 1 of Candide or Optimism, by Voltaire, recorded for LibriVox.org by Bern')

Observe that AzureOpenAI client behavior does not depend on the library function used.

token_provider = get_bearer_token_provider(DefaultAzureCredential(), "https://cognitiveservices.azure.com/.default")
azure_endpoint = os.environ['AZURE_ENDPOINT']
azure_deployment = "whisper/audio/transcriptions?api-version=2024-06-01"

azure_client = AzureOpenAI(
    azure_ad_token_provider=token_provider,
    azure_endpoint=os.environ['AZURE_ENDPOINT'],
    azure_deployment=azure_deployment,
    api_version="2024-06-01"
)

with open(audio_file, 'rb') as f:
    result = azure_client.audio.transcriptions.create(
        file=f,
        model="whisper-1",
    )

    print(f"azure_client.audio.transcriptions: {result.__class__.__name__, result.text[:60]}")

    result = azure_client.audio.translations.create(
        file=f,
        model="whisper-1",
    )

    print(f"azure_client.audio.translations: {result.__class__.__name__, result.text[:60]}")

azure_client.audio.transcriptions: ('Transcription', "Chapitre premier de Candide ou l'optimisme, de Voltaire, enr")
azure_client.audio.translations: ('Translation', "Chapitre premier de Candide ou l'optimisme, de Voltaire, enr")

azure_deployment = "whisper/audio/translations?api-version=2024-06-01"

azure_client = AzureOpenAI(
    azure_ad_token_provider=token_provider,
    azure_endpoint=os.environ['AZURE_ENDPOINT'],
    azure_deployment=azure_deployment,
    api_version="2024-06-01"
)

with open(audio_file, 'rb') as f:
    result = azure_client.audio.transcriptions.create(
        file=f,
        model="whisper-1",
    )

    print(f"azure_client.audio.transcriptions: {result.__class__.__name__, result.text[:60]}")

    result = azure_client.audio.translations.create(
        file=f,
        model="whisper-1",
    )

    print(f"azure_client.audio.translations: {result.__class__.__name__, result.text[:60]}")

azure_client.audio.transcriptions: ('Transcription', 'Chapter 1 of Candide or Optimism, by Voltaire, recorded for ')
azure_client.audio.translations: ('Translation', 'Chapter 1 of Candide or Optimism, by Voltaire, recorded for ')

Code snippets
No response
OS
Ubuntu
Python version
Python v3.12.7
Library version
openai v1.55.3

cc @kristapratico

@s-zanella can you share the reason for including the full path + API version ("whisper/audio/translations?api-version=2024-06-01") in the azure_deployment parameter? It is expected that only the deployment name, i.e. whisper, is passed and the client will build the URL.

I see. https://{endpoint}/openai/deployments/whisper/audio/translations?api-version=2024-06-01 is the endpoint URI that shows in the Azure OpenAI Service portal. Usually for other services, copy & pasting this and passing it as the azure_endpoint works. This is what I did initially; the code above is refactored to use azure_endpoint and azure_deployment parameters and in retrospective makes it more evident that I should have used azure_deployment = "whisper" .
I tried removing the API version and using whisper/audio from the original URI but neither worked. Using just whisper works as expected. Still, the behaviour is puzzling and I feel that there should be a guard against using an endpoint that does not match the library function.


Transfer a repo

Related to newly approved lab hyperledger-labs.github.io#238

Please include:

GitHub repo to transfer: https://github.com/AdharaProjects/Crosschain-Interop
Lab to be added to: This will become the new lab harmonia
Teams with which access: This will be a new team

Team name: harmonia-committers
Role:

Manage

Team members:
- https://github.com/coeniebeyers - Adhara
- https://github.com/eierina - R3
- https://github.com/fenryka - R3
- https://github.com/gendal - R3
- https://github.com/MarkOldfield - R3
- https://github.com/MicheleCullinan - Adhara
- https://github.com/mkrielza - Adhara
- https://github.com/petermunnings - Adhara
- https://github.com/r3domfox - R3

The team is created; once the repo is transferred, I will make the change

@petermunnings you don't need to wait on the PR merge - it will come over. go here and transfer it to me, @ryjones.

Done:  "Repository transfer to ryjones requested"


Could somebody provide afFormGroup code example?

Was trying to use afFormGroup method to show two fields in one line and failed.
Here is the code I am using for the schema:
  matching:
    type: [Object]

  "matching.$.frontend_prefix":
    label: 'Frontend Prefix'
    type: String

  "matching.$.backend_prefix":
    label: 'Backend Prefix'
    type: String
    regEx: /^[a-z0-9A-Z_]{3,15}$/

@aldeed any thoughts on this?


Use ExceptionTelemetry for tracking exceptions

Tracking ExceptionTelemetry instances rather than TraceTelemetry provides a nicer experience in UI's such as Visual Studio and the Azure portal.
Resolves #393

@henkmollema,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.
It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@henkmollema, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.
Thanks, Microsoft Pull Request Bot

Thanks @dnduffy! Any chance this makes it into 2.1?

@henkmollema We actually had the final build for 2.1 on Wednesday and just released it today after a final test pass.  However, there is talk of pushing a couple features into a 2.1.1 before 2.2 so it might not wait very long for inclusion in a stable release.

I see, no worries. Thanks!


Load Pdf Files Directly

hello, is it posible to load files directly without the need to load them manually trough an input form?

Hi.
Sorry, English can understand only 3-year-old level.
$B3N$+!"(BUint8Array$B$GFI$_9~$s$G$$$k$O$:$G$9$N$G!"(B
Uint8Array$B7A<0$G$"$l$P2?$G$b(BOK$B$G$9!#$?$V$s(B :-)
Anything is OK as long as it is Uint8Array object. maybe :-)

hello, is it posible to load files directly without the need to load them
manually trough an input form?
You are receiving this because you are subscribed to this thread.
Reply to this email directly, [a:https://github.com/TakeshiOkamoto/
PDFDesigner/issues/1]view it on GitHub, or [a:https://github.com/
notifications/unsubscribe-auth/APynjtL2IasK_9h_bfMTuDzAlIqehn7_ks5rVRFGgaJpZM
4Lrhb1]mute the thread.[img:https://github.com/notifications/beacon/
APynjqpxCVdwvurvdbPBUDdmicvZ9Bmiks5rVRFGgaJpZM4Lrhb1.gif]

ok, don´t worry, your library is amazing ! thanks for sharing this solution. I´ll find the way to resolve this issue. have a great one =)

こんにちは、あなたはそれを使用する方法の例で私を助けることができた、例のリンクは動作しません、事前に助けてくれてありがとう。
Kon'nichiwa, anata wa sore o shiyō suru hōhō no rei de watashi o tasukeru koto ga dekita, rei no rinku wa dōsa shimasen, jizen ni tasukete kurete arigatō.
Hi, you could help me with an example of how to use it, the example links do not work, thank you in advance for the help.

Sorry, We are currently unable to access the site because the server is down.
how to use
https://www.petitmonte.com/pdfdesigner/en/howto.html
Please wait for a while as the rental server is under recovery work.


Redirect URI doesn't match your registered redirect URIs - ORCID

I have an issue on the ORCID authentification server.
Get this error from the ORCID production server: Redirect URI doesn't match your registered redirect URIs.
Pretty sure that the callback URI registered on the server match to the one registered in the client APP.
I suspect that the server is not refreshing the cache instantaneously which result to this error.

Fixed by refreshing app credential on ORCID server

This issue popped up again today. I got the redirection working fine and after two trials it starts showing the message "Redirect URI doesn't match your registered redirect URIs". The URI is same as what it was before.


Errors while running

The following errors occur for the program when I run it. Previously it was running perfectly fine but now it doesn't get corrected.
Does anyone have any ideas on how to correct these?
Traceback (most recent call last):
  File "C:\Users\Admin\Desktop\HTC\python-evtx\scripts\evtx_filter_records.py", line 65, in <module>
    main()
  File "C:\Users\Admin\Desktop\HTC\python-evtx\scripts\evtx_filter_records.py", line 56, in main
    for node, err in xml_records(args.evtx):
  File "C:\Users\Admin\Desktop\HTC\python-evtx\scripts\evtx_filter_records.py", line 30, in xml_records
    yield to_lxml(xml), None
  File "C:\Users\Admin\Desktop\HTC\python-evtx\scripts\evtx_filter_records.py", line 15, in to_lxml
    record_xml)
  File "src\lxml\etree.pyx", line 3211, in lxml.etree.fromstring
  File "src\lxml\parser.pxi", line 1872, in lxml.etree._parseMemoryDocument
ValueError: Unicode strings with encoding declaration are not supported. Please use bytes input or XML fragments without declaration.

what changed since the last time it was running fine?

nothing actually. I didn't run it for a few days and the next time I did, it shows these errors

Hey, anyone else who had this issue? Did it get rectified??

Add .encode('utf-8') at line 15 file evtx_filter_records.py after record_xml like so:
    return etree.fromstring("<?xml version=\"1.0\" encoding=\"utf-8\" standalone=\"yes\" ?>%s" %
                         record_xml.encode('utf-8'))

This fixes the error.


Validation of Content-Type Header

As a Product Team, we need to ensure file upload functionality has strong security controls.
Acceptance Criteria

[ ] check the content-type header on uploads and only accepting valid strings which match the allowed extensions.
[ ] force the extensions of the url and uniquefilename fields to match that of the actual filename
[ ] disable file upload on cancelled application

Context:
We note that the file is assigned a unique random name internally which does not match the original file name (a good practice), but the original name is kept and re-used later while downloading. The application does not verify the mime-type (in the Content-type header), only the file extension (.png), so any content can be uploaded if the extension is acceptable.

@andrewsignori-aot to add technical details on this one.

Close as obsolete. Also confirmed validation on mime types.


.oh-my-zsh/lib/misc.zsh:3: parse error near then'` after update

Not a critical issue.
Upon updating oh-my-zsh, I get the pretty Hooray! message, with the appropriate twitter and swag links. Immediately thereafter, I get hit with a:
/path/to/user/.oh-my-zsh/lib/misc.zsh:3: parse error near 'then'
Here's that file - I don't see any immediate errors.
## Load smart urls if available
for d in $fpath; do
	if [[ -e "$d/url-quote-magic"]]; then
		autoload -U url-quote-magic
		zle -N self-insert url-quote-magic
	fi
done

## jobs
setopt long_list_jobs

## pager
export PAGER="less"
export LESS="-R"

## super user alias
alias _='sudo'
alias please='sudo'

## more intelligent acking for ubuntu users
alias afind='ack-grep -il'

# only define LC_CTYPE if undefined
if [[ -z "$LC_CTYPE" && -z "$LC_ALL" ]]; then
	export LC_CTYPE=${LANG%%:*} # pick the first entry from LANG
fi

# recognize comments
setopt interactivecomments

It should also be noted the error gives me a ` as opposed to a ' before the 'then'.

Yep - having the same issue.

On Line 3, if [[ -e "$d/url-quote-magic"]]; then there is a missing space between " and ].
It should be like this if [[ -e "$d/url-quote-magic" ]]; then. This fixed it for me and my buddy.

Thanks for reporting.
This is fixed in #4002 and many other PRs, and originally reported in #3986.

I have this issue on like 5 computers super annoying. :disappointed:

+1

+1 having the same issue

Same here.

duplicate of https://github.com/robbyrussell/oh-my-zsh/pull/4002
please close.

@leee , can you close this issue? It should be fixed in the master already by #4002

+1 having the same issue

If you are still experiencing the issue, please get the latest version using upgrade_oh_my_zsh.

Sorry for the delay, I never have time to contribute nowadays. Appears to be good, closing.

Thanks!


(cleanup) drop header lines

I believe we don't need file encoding lines in python 3 since the default is utf8, and don't need shebangs for files that don't make sense to mark executable. Just a quick cleanup PR -- no big deal to close if you want to keep these around for whatever reason.

I swear I yanked these from CL not too long ago. Py2....it pulls you back in.

I swear I yanked these from CL not too long ago. Py2....it pulls you back in.


Simplify psibase CLI - Remove no-doc option

[ ] #489

Since the docs are now added to psinode by default, I also took some time to clean up how they work. I made doc-sys into an applet, and improved the synchronization between URL changes within the iframe and the displayed URL in the web browser to better conform to user expectations.
I also added the docs to the "home page" for now, and removed the "about" link which was broken on local deployments anyway.

The default theme I'm seeing with the docs in dark mode but a white background for the common navbar is a bit jarring.

Definitely agree. I think nav bar should be generally themeable, and default dark mode.


Champion Planning – Cohort 1

The ultimate vision of Openscapes is that we can get to environmental solutions faster if we are more efficient and collaborative in how we do science.
The Openscapes Champion* program is a mentor program for early career researchers. Its main goal is to help Champions and their lab** work more efficiently using existing open tools. The program will be a mix of one-on-one meetings with Champions and Julie, and community calls with all Champions and members from their labs. This way, it's possible to discuss and plan specifically for individual Champions, have concrete progress from their labs, and connect and learn with other Champions and their labs. This will strengthen the community of practice around open, collaborative environmental science.
*name may change
**“Lab” is broadly defined; it’s whatever group you work with or will be working with in the future. It can even be Future You.

Correspondence Summary

[x] Sept 28. Email sent to pilot cohort to introduce the idea of Openscapes and invite participation

[x] participation confirmed

[x] Nov 2. Emails sent to individual Champions with more details and asking to book an appointment slot for November call

Openscapes Planning – Name
Emailed Nov 2, lightly edited
TL;DR: I’m excited to start working with you through Openscapes! The program will be from Jan-May and be done remotely. Please book an hour call with me in November to discuss.
Openscapes Plan
I am so excited to work with you through Openscapes! I have been formalizing the plan for the program that will be from Jan-May 2019. And I am ready to meet with you to introduce my ideas and discuss your needs.
The ultimate vision of Openscapes is that we can get to environmental solutions faster if we are more efficient and collaborative in how we do science. The main goal of the “Championship” program is to help you and your lab* work more efficiently using existing open tools – while engaging you but requiring the least amount of time from you personally. The program will be a mix of one-on-one meetings with you and me, and community calls with all Champions and members from their labs. This way, we’ll be able to discuss and plan specifically for you, have concrete progress from your lab, and connect and learn with other Champions and their labs.
Details & Timeline for 2019
We will have 3 scheduled meetings (total of 4 hours) each month from Jan-May 2019. (Yes, it’s OK if you miss some).
| One-on-one mentoring calls | Community calls
-- | -- | --
Purpose | Discuss and strategize: how do you work, identify how to be more efficient, set/review/meet goals | Learn about existing tools & practices and how to engage, meet other labs and see commonalities, break-out groups (split Champions and lab members) to discuss steps forward + keep accountable; have fun!
Outcomes | Increase personal efficiency in at least one concrete way, enable your lab/department, become a Champion for open practices | Increase your lab’s efficiency in several concrete ways, build community and skillsets in your department and beyond
Process | Frequency: 1x per month, 1 hourParticipants: you, JulieMedium: Zoom, Google DocsAdditional possibility: Julie joins one lab meeting per month remotely | Frequency: 2x per month, 1.5 hours Participants: all Champions and their lab members, JulieMedium: Zoom (breakout rooms), Google Docs
November Intro Call:
I’d like to meet with you for an hour with this agenda:

Intro to open practices, get you as excited as I am!
Understand how you/your lab works (I’ll have prompting questions)
Start framing goals for you/your lab (will help me plan)
Answer your questions
Logistics:

Discuss which lab member to include (get contact info)
Schedule repeat one-on-ones, ballpark times for community calls, seminar

Next Steps:
Book a 1-hour intro call with me in November here. Time slots are available across all weeks in November; if none of them work for you please let me know and we’ll figure out another time.
I’m so excited to work with you, and please let me know if you have any questions!
Cheers,
Julie
* “Lab” is broadly defined; it’s whatever group you work with or will be working with in the future. It can even be Future You.


common_options.h: What type for UserAgentPrefixOption?

This comment from @coryan https://github.com/googleapis/google-cloud-cpp/pull/5949#pullrequestreview-601260170

Maybe a followup PR, I am not sure what the right representation for the UserAgent header should be. I think the following properties are desirable:

Applications can add their own.
Intermediate libraries (think something like Tensorflow, built on top of our GCS library) could add their own.
Applications cannot remove the prefixes added by libraries (because we lose the ability to troubleshoot things).
The end result is a string that we put in some HTTP header (or a similar thing).

Seems like order should matter, maybe a std::vector<> is better than std::set<>, seems like it will always be "small", maybe a dozen elements. Seems like a std::string could handle this just fine.

According to https://tools.ietf.org/html/rfc7231#section-5.5.3, it looks like order is important: "By convention, the product identifiers are listed in decreasing order of their significance for identifying the user agent software." So the original choice of std::set was not good.
Uniqueness seems somewhat important, because what's the point of listing the same user-agent multiple times? However, it's likely easy enough to just rely on folks simply not setting it multiple times. That is, I think I agree now that the uniqueness offered by std::set is not crucial.
Regarding "Applications cannot remove the prefixes added by libraries", I'm not sure if/how we can avoid that one for certain. Other than to say that user-agents can only ever be added by a library that has access to the internal::Options instance that's eventually passed to MakeConnection(). Our user-agent string will be added within MakeConnection(), so nobody could ever remove ours. This all seems fine to me.
I agree that a std::string could work. One concern with std::string is that it doesn't capture the fact that multiple "product identifiers" (terminology from RFC 7231) can be added. I know we use the term "prefix", but it would be easy for a user to accidentally set the string to their product, thus erasing previously set product identifiers. Whereas, if we used a type that naturally conveys the fact that there can be multiple user-agent product identifiers, it may be less likely that users accidentally erase others' product IDs. Additionally "appending" one's product ID to a user-agent std::vector (or better yet, std::deque to allow prepending) would be even easier for users since they wouldn't need to worry about concatenating, and joining with a space.
Proposal
So given all that, I think I agree that std::set was the wrong choice:
https://github.com/googleapis/google-cloud-cpp/blob/37054c009d74917d04cabe40aa796be9950e8ed2/google/cloud/internal/common_options.h#L46-L48
However, now I think std::deque may be preferred.
Thoughts?

However, now I think std::deque may be preferred.

std::deque would work. Alternatively, std::vector<> using .push_back() to add and .rbegin()/.rend() to iterate would work too?

Our user-agent string will be added within MakeConnection(), so nobody could ever remove ours. This all seems fine to me.

That seems like a good place to start, and most likely all we will ever need.


fix(table): make column headers bold

fixes https://github.com/patternfly/patternfly-next/issues/1947

Deploy preview for pf-next ready!
Built with commit d351ccc7bf7e0af69700ba2bc918c652e5a87768
https://deploy-preview-1949--pf-next.netlify.com

:tada: This PR is included in version 2.13.3 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


Performance problem.

I'm currently writing a program which will reencode nearly every instruction of a big problem(a game).
So there will be millions of calls on reencode callback. (see here.)
The problem is that if I declare the Program and Assembler as local variables, then there will be millions of object/memory allocations and deallocations which will be a big impact on speed.
But if I declare them as global variables, I didn't see any clear methods so the generated code will be accumulated on every call.
Is there any solutions to this?

You can destroy the nodes via program.destroy(node);, clear would be the equivalent of traversing all nodes in the program and calling destroy. But having a clear method is definitely wanted.

Also calling serialize multiple times will overwrite the previous generated code so you can re-use it as it is.

I've added a clear method to Program in https://github.com/ZehMatt/zasm/commit/bfc3a16ac3d71ee6633beaeecdd7fd79fd37fbc1
If this is what you wanted feel free to close the issue.

I've added a clear method to Program in bfc3a16
If this is what you wanted feel free to close the issue.

Wow, thanks for you reply in such a short time.
The patch did what I want for sure. Thank you!

I'm sorry to reopen this.
After your fix, I still feels the program runs pretty slow, so I did some benchmark on popular asm jit encoders.
And I got the following result:
Average output for test several times:

Debug Build:
Asmjit 79.5586 ms
Xbyak 268.761 ms
Zasm 34107.5 ms

Release Build:
Asmjit 7.0578 ms
Xbyak 13.9802 ms
Zasm 6339.41 ms

zasm get unexpected performance compared to the other two engine.
I'm not 100% sure I use there library correctly, but all I want is to get new generated code buffer on each loop.
Can you give me some advice?
The test code can be found here.
I upload the benchmark project for anyone who wanna a try.

I add Zydis encoder test.
Seems Zydis encoder itself is acceptable.
CPU: Intel i7-8700
Average output for test several times:

Debug Build:

Asmjit 77.5361 ms
Xbyak 206.549 ms
Zydis 88.2653 ms
Zasm 31610.4 ms

Release Build:

Asmjit 6.3319 ms
Xbyak 11.3565 ms
Zydis 25.2644 ms
Zasm 6156.57 ms

I've tried to reproduce the issue where getCode() and adapted a test for this. Unfortunately I can't reproduce the issue, the test looks like following: https://gist.github.com/ZehMatt/1b45af7ac60b076ff97eccb82acd017f
Regarding performance, I didn't get to optimize anything just yet, keep in mind that zasm is a general purpose assembler while zydis encoder is just purely an encoder, zasm implements labels and sections and also stores instruction as nodes to allow re-ordering so there is an added cost. I want to look into optimizing it a bit at some point but right now I'm still more focused on adding essential features and having the general API as user friendly as possible.

I also just realized you are testing the Assembler in asmjit, to have a fair comparison you would need to compare asmjits Builder.

I just merged a PR where I did some smaller performance improvements. To keep in mind I'm not trying to compete with asmjit, this is more focused towards tooling where performance is not as super important.

OK, I see.
zasm is designed as a full featured assembler, not a jit asm compiler, which is what I need.


caf script should capture MPI scripts with full paths

I installed OpenCoarrays like this:
# MPICH_* override because MPICH was built against GCC-5
export MPICH_CC=gcc-6
export MPICH_FC=gfortran-6
export MPI_DIR=/opt/mpich/dev/gcc/default # Jeff's Mac
git clone https://github.com/sourceryinstitute/opencoarrays.git && cd opencoarrays && \
mkdir build && cd build && \
CC=$MPI_DIR/bin/mpicc FC=$MPI_DIR/bin/mpifort cmake .. \
-DCMAKE_INSTALL_PREFIX=/opt/opencoarrays/mpich-dev-gcc-6 \
-DMPI_C_COMPILER=$MPI_DIR/bin/mpicc -DMPI_Fortran_COMPILER=$MPI_DIR/bin/mpifort && \
make -j4 && \
ctest && \
make install

When I went to use caf, I saw this error:
github-official jrhammon$ make allfortran
cd FORTRAN/Synch_p2p;       /Applications/Xcode.app/Contents/Developer/usr/bin/make p2p
/opt/opencoarrays/mpich-dev-gcc-6/bin/caf -std=f2008 -ffree-form -cpp -O3  -DPRKVERSION="'2.16'" -DRADIUS=  p2p.f90 -o p2p
ifort: command line warning #10006: ignoring unknown option '-std=f2008'
ifort: command line warning #10006: ignoring unknown option '-ffree-form'
ifort: command line warning #10006: ignoring unknown option '-fcoarray=lib'
^Cmake[1]: *** [p2p] Error 2
make: *** [allfortranserial] Interrupt: 2

This is because I have mpif90 associated with ifort in my PATH.  I rely upon absolute paths to get GCC and LLVM MPI wrappers, as I did above.
Unfortunately, caf doesn't remember what MPI wrapper was used to build the library:
# Default to "mpif90" Fortran compiler if environment variable CAFC has zero length:
if [ -z "$CAFC" ]; then
   CAFC=mpif90
fi

It really should do this, both because of people like me who don't necessarily have the right mpif90 first in their PATH on purpose, but also for anyone that doesn't know better and switches MPI installation.
This will likely prevent all sorts of awful bug reports associated with people that compile with MPICH but try to run with Open-MPI, and vice versa.

I agree, and this has been on my to-do list for quite some time. I'll dig into it on Wednesday or Thursday

@jefhammond, great catch.  Thanks for all the fabulous feedback.  Keep it coming.
A quick fix is to insert a line of the following form at the top of the caf script:
source <opencoarrays-install-dir>/setup.sh

which will prepend absolute paths in the PATH environment variable for the process executing caf.  The setup.sh file (or setup.csh for C-shells) contains source statements for all OpenCoarrays-installed software.  I think it correctly reflects any pre-installed software the user specified with in the way you did by your setting CC and FC.  I'll verify this and make sure user choices are respected.  I'll aim to have this done shortly.  I'll see if we can get  a bug fix release out this week that reflects some or all of your feedback. If so, it would be great if you could test it for us.

  However, I will be happy to test.

@jeffhammond I could look at the CMake problems if you think I could be helpful...   I think I fixed it, it just required 3x more work than is reasonable.

@jeffhammond OK. Just curious, are you using the find_package capability?
Yes CMake is not designed so well... but I have yet to find an alternative that I like more. I'd love to check out Meson at some point and see if it's a bit saner, but am nervous about its relative new-ness and stability.

@zbeekman
I am just invoking CMake with CC=$MPI_PATH/bin/mpicc and foolishly expecting that CMake can figure out that the libraries it should use are in $MPI_PATH/lib instead of some other location.
I cannot prove Autotools is broken, so I have not tried to fix it.  And yes, not having Windows support is a feature of Autotools in my book 😉

My earlier suggestion was based on a mistaken assumption that our install.sh script launched the build. I realized a little while ago that the scenario described in this issue involves a user launching a build by directly invoking CMake. The previously proposed solution would not have worked because the aforementioned setup.sh file is written by install.sh.
Pull request #197 fixes this issue by directly modifying the CMake script that builds the "caf" compilation script. Hence, the fix should work whether a user installs via our installation script or simply via CMake directly.
I'm closing this issue.  We can reopen it if #197 fails.

@rouson If you say "fixes #193" in the merge commit comments (possibly in the pull request description as well) it will automatically close this issue once it gets merged, just FYI. closing issues with commit messages

@jeffhammond I believe this issue is resolved in the latest release and the code in master. Please let me know if you find otherwise.

Thanks.  Will test next time I do a local build of PRK.


Setting the full model range as editable breaks completions

To reproduce:

go to https://microsoft.github.io/monaco-editor/playground.html#extending-language-services-configure-json-defaults
add editor.getModel().setEditableRange(editor.getModel().getEditableRange()); as the last line
run
add a new empty line at the beginning of the document
apply a completion at the beginning of the document

Only the part after the inserted text is editable when the whole document should be editable.

The model editable range uses two markers for tracking.
The root cause is that applying a suggestion sends an edit with forceMoveMarkers: true. That causes all markers sitting at that position to move, including the editable range start marker. I recommend listening to model content change events, reading the editable range and if it has moved unexpectedly, to adjust accordingly (i.e. recompute it on your side).


Starting gradlew jettyRun permanently

Hi,
The docs describe how to start the monkeys using gradlew jettyRun.  But how do I start that so it runs permanently as a service?
Thanks,
-T

You could use the generated war file and deploy it to tomcat instead. Use
./gradlew build to generate the war file