Remote Debug fails for projects that multi target several frameworks (Error NETSDK1129)

The remote debug fails for projects that multi target several frameworks added in the csproj:
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netcoreapp3.1;net5.0</TargetFrameworks>
    <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
  </PropertyGroup>

</Project>

Trying to debug generates an error message

[raspberry]: INFO: Checking for native Windows OpenSSH client
[raspberry]: INFO: Building: D:\FooApp\FooApp\FooApp.csproj
[raspberry]: INFO: Build succeeded
[raspberry]: INFO: Publishing: D:\FooApp\FooApp\FooApp.csproj
[raspberry]: ERROR: Publish failed: ExitCode=1
Microsoft (R)-Build-Engine, Version 16.10.1+2fd48ab73 für .NET
Copyright (C) Microsoft Corporation. Alle Rechte vorbehalten.
  Wiederherzustellende Projekte werden ermittelt...
  "D:\FooApp\FooApp\FooApp.csproj" wiederhergestellt (in 89 ms).
C:\Program Files\dotnet\sdk\5.0.301\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.CrossTargeting.targets(27,5): error NETSDK1129: Das Ziel für "Publish" wird ohne Angabe eines Zielframeworks nicht unterstützt. Das aktuelle Projekt verwendet mehrere Frameworks als Ziel.  [D:\FooApp\FooApp\FooApp.csproj]

when I only use 1 targetframework, debug works

any update? When I try to debug the extension from source code I only get a ghosted VS2019 window and VS is not responding

ok, on a second VM debugging works and I can see in PublishProjectAsync the --framework option is not set.
So I added a property Framework to ProjectProperties and set it in CopyFrom to value of FriendlyTargetFramework, but this is always the first value from TargetFrameworks and not the value from the debug settings of the project.

ok, I fixed it in my local copy. I need to use IActiveDebugFrameworkServices to get information about debugging info (configuration, platform and targetframework).

And when I pass this retrieved targetframework to dotnet publish command in DebugHelper, the publish and debug works.

But there is an issue. The stable nuget package Microsoft.VisualStudio.ProjectSystem.Managed 2.0.6142705 on nuget.org misses the required Interface  IActiveDebugFrameworkServices and I to use the pre-release feed on azure  to get the 16.10 version.
Now there is the question what to do? Is it ok to use a lot of pre-release packages?

any update @jefflill ? Should I create a PR by using a lot of pre-Release nugets from Azure feed? I added a nuget.config wit the feed so build will work.

@jefflill are you working again on the extension?

Yeah, I’ve been thinking about releasing the VS 2022 support pull request,
but I’ve been busy with another project.
Want to help?
—Jeff
On Mon, Dec 18, 2023 at 11:56 PM MagicAndre1981 @.***>
wrote:

@jefflill https://github.com/jefflill are you working again on the
extension?
—
Reply to this email directly, view it on GitHub
https://github.com/nforgeio/RaspberryDebugger/issues/36#issuecomment-1862280605,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABFOLD37JENTJELNFOZC5HLYKFCBPAVCNFSM46Y3ISVKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBWGIZDQMBWGA2Q
.
You are receiving this because you were mentioned.Message ID:
@.***>

where you able to get the 2022 versions of my added libs? The user who forked it and added VS2022 support reverted my PR because we was not able to get the prerelease libs from private nuget feeds. I haven't checked if those feeds are still valid.


Ensure IS NULL does not match empty arrays

Summary of the changes / Why this improves CrateDB
On an empty array there are no values in the index, causing the
FieldExistsQuery to not match.
That resulted in inconsistent behavior:
cr> select *, xs is null from tbl;
+--------+--------------+
| xs     | (xs IS NULL) |
+--------+--------------+
| NULL   | TRUE         |
| [1, 2] | FALSE        |
| []     | FALSE        |
+--------+--------------+

cr> select *, xs is null from tbl where xs is null;
+------+--------------+
| xs   | (xs IS NULL) |
+------+--------------+
| NULL | TRUE         |
| []   | FALSE        |
+------+--------------+

The solution is to add a GenericFunctionQuery in addition.
Unfortunately this adds some performance overhead if there are a lot of
NULL values/empty arrays.
An alternative would be to adjust both query and index code to always
use the _field_names index for arrays, but that has the disadvantage
of only working for new data.
Checklist

[x] Added an entry in CHANGES.txt for user facing changes
[x] Updated documentation & sql_features table for user facing changes
[x] Touched code is covered by tests
[x] CLA is signed
[x] This does not contain breaking changes, or if it does:

It is released within a major release
It is recorded in CHANGES.txt
It was marked as deprecated in an earlier release if possible
You've thought about the consequences and other components are adapted
(E.g. AdminUI)

An alternative would be to adjust both query and index code to always use the _field_names index for arrays, but that has the disadvantage of only working for new data.

Can't we have both solutions?

Can't we have both solutions?

We could do the _field_names thing additionally, but only for new tables so that we know for sure the field is present. But it would also have a storage impact. I'm not sure if it is justified.

Then I agree to not do it.


Add and remove

In this project,

I implement the CRUD (create, update, delete) methods. All the elements of the user interface will be fully functional and my application is completed.
to delete an element you should select it first.

I deploy it on netlify because my github forces me to create another pull request that is not allowed in this project. but the god part of the project I will deploy it on github because it will contain a new pull request


Error reading Postgis - Geometry column #93

Could someone see why the tests are failing?

Ugg sorry I looked into this a while back and made a little progress but couldn’t ultimately understand why it is failing in CI environment but was working locally. Then I got sucked into a bunch of local fires here.  I tried a few different docker configurations and could t work out what had changed to cause them to fail after working for a while.  Might be worth starting a new ticket on this and I can try to recall what I tried before getting distracted
Von meinem iPhone gesendet

Am 24.06.2020 um 3:34 AM schrieb Sandeep Chandra<EMAIL_ADDRESS>
Could someone see why the tests are failing?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

@daz10000 would it be possible for you to test my changes locally on your computer? If they pass could you please publish new nuget as I am blocked because of this issue.

@daz10000 I think it might be something to do with global.json, I 'll create a PR for it to see how it goes.

On Thu, Jun 25, 2020 at 02:14:49AM -0700, Sandeep Chandra wrote:

@daz10000 would it be possible for you to test my changes locally on your computer? If they pass could you please publish new nuget as I am blocked because of this issue.

Sorry to hear that - can take a look next hour or so.

Should be merged in 0.2.10-beta.   I haven't done a release on this project for a little while so let me know ASAP if I f&*ek anything up but should be good.

Thanks @daz10000 , it is working fine.


new config options

allow disabling dying mobs (on by default)
disable better slime particles (off by default)

disabling slime particles would be extremely difficult, since the fabric conditional data API seems to only have options for loading based on what mods are resolved. I would have to do dynamic data generation, which is a lot of work. maybe i can come back to this later


How does flutter dynamically request a whitelist from the system to keep it alive?

I want to develop an IM mobile app to ensure that it can run in the background.

You should ask this question on stack-overflow not in github issues. For your question, you can use isolate if it call from foreground and AndroidAlarmManager in background.

Thanks.


Flakes/NixOs support

Would be lovely if you could include flakes/NixOs support. Thanks for your work!

Yes this would be great, might be able to copy the configuration of the original split workspaces and modify it a bit


Front to back rendering

[ ] Depends on https://github.com/ppy/osu-framework/pull/251

Please don't merge yet.
I was convinced we can avoid filling fragments once the topmost ones reached an alpha value of 1, thus avoiding lots of wasted fill-rate on things like glow that is mostly hidden behind drawables. Turns out in order to make this actually work with alpha values we need some GLSL features from OpenGL 4.2.
Thus, the speed of this branch is comparable with master. It has some simplifications and some things that become more complicated. Feedback is welcome.

Closing for now (a lot of this has already been cherry-picked; remaining implementation is trivial).


Switch to poise

Switch to poise for registering commands
If we choose to use this then this closes #17

commit 39e544b9230b8c8a728865d81294ec2de4a0a069 (HEAD -> poise-setup, origin/poise-setup)
Author: Your Name<EMAIL_ADDRESS>Date:   Sun Nov 19 21:08:22 2023 +0000
remove unused pub

please consider changing your name from "Your Name" to your actual name

This looks cleaner, it's missing the ephemeral option on replies but I'm in favor of changing to Poise.


Add GitLab engine

Hi,
this PR adds the ability to search Gitlab merge requests.

Great, thanks! I tweaked trimLines a bit so that it'll show maxRows rows (instead of halfRows rows) when the query is in the result's first line.


adds two unit tests for JctFileManagerImpl module.

Description
adds two Junit tests for JctFileManagerImpl module. Kindly consider this as my initial draft
related to #219

I've enabled the CI pipeline runners for you, btw.

Codecov Report

Merging #232 (8c10a8a) into main (8d8ecbc) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #232   +/-   ##
=======================================
  Coverage   68.52%   68.52%
=======================================
  Files          72       72
  Lines        2532     2532
  Branches      233      233
=======================================
  Hits         1735     1735
  Misses        731      731
  Partials       66       66

Thanks for the review comments and thank you so much for your patience

@Badrri-Narayanan no worries at all.
LGTM.


Create wildcard_matching.cpp

Added wildcard matching problem, solved using backtracing
Checklist

[x] Added description of change
[x] Added file name matches File name guidelines
[x] Added tests and example, test must pass
[x] Added documentation so that the program is self-explanatory and educational - Doxygen guidelines
[x] Relevant documentation/comments is changed or added
[x] PR title follows semantic commit guidelines
[x] Search previous suggestions before making a new one, as yours may be a duplicate.
[x] I acknowledge that all my contributions will be made under the project's license.

Notes:

any particular reason this PR was closed again?

I mistakenly added multiple changes into this commit. I have created a new one.


Support for module released as pre-release

When modules repo releases are marked as pre-release they aren't discovered by module add

this issue will be addressed with a "/module options" sub-command. one of these options will be the release channel to use for "/module add" and "/module update"

The /module channel sub-command now takes care of this.


v0.14.0 Release

Updated Get Blob Tags and Set Blob Tags function signatures
Added Put Blob From URL
Offer knob to disable application logging (Syslog)
Added examples for MSI Login
Updated go.mod to address dependency issues
Fixed issues #260 and #257

There seems to be a flaky test, I triggered a rerun


feat(storage): update dual-region sample using CustomPlacementConfig

Fixes #2582
Tests pending https://github.com/googleapis/google-cloud-go/pull/6294 and new config to roll out by July 6th

@cojenco does this PR need a new library version before it's ready for review?

@cojenco does this PR need a new library version before it's ready for review?

Yep this PR requires  a new library release once the feature changes are merged in, thanks!


Injectable experiment searcher

this PR replaces the definition of the experiment searcher as commented at #7

Coverage remained the same at 90.336% when pulling 949b6089021df4a5abe5da590dbfec36f6162518 on kpacha:searcher into 95480ddb1abf4bb099f01e379070596521bb878b on klokare:master.


No values in Settings gives an error

If my settings are all deleted like this:

Then this Notice shows up:

It'd also be great to work in these conditions with the default values.
Note: I would like to work on this issue, also please add a bug label to it.

If you don't mind, I'll try implementing this myself.

Alright, no problem

Feature implemented in commit 0427919.


Fix marginalization during prediction

Marginalization should be performed on an float32 image with values in
the interval [0,1], followed by normalization.

@feihugis can you try this out?

@dusenberrymw LGTM!


Can't turn on/off my 400s in homekit.

Describe The Bug
I can't see the state and I can't turn on/off my levoit 400s within the homekit app on my iphone running IOS 15.5.
here's what I tried :
turning the air purifier on/off with the VeSync App (it does work).
turning the air purifier on homebridge (in the "accessory" tab", and it does work)
rebooting my iphone. (doesnt work)
rebooting my raspberry pi. doesnt work.
I tried unpairing the child bridge and pair it again. doesnt work.
I tried unplugging/plugging the air purifier, lol.
I also tried to empty the cached data in homebridge settings. doesnt work.
Note : I forgot to mention that it was working correctly this morning. (its not username or password related..!)
Here are the logs :

Expected behavior
Air quality should display, and I should be able to turn on/off the air purifier.
Logs
see picture above
Plugin Config
{
"bridge": {
"name": "Homebridge,
"username": "XXX",
"port": 51991,
"pin": "XXX",
"advertiser": "avahi",
"bind": [
"eth0"
]
},
"accessories": [],
"platforms": [
{
"name": "Config",
"port": 8581,
"platform": "config"
},
{
"name": "LAPoB",
"email": "XXX",
"password": "XXX",
"platform": "LevoitAirPurifiers",
"_bridge": {
"username": "XXX",
"port": 50789
}
}
]
}
Plugin Version
homebridge-levoit-air-purifier v1.1.13
Homebridge Version
v1.5
Node.js Version
v16.15.1
NPM Version
?
Operating System
Official Homebridge for raspberry pi Image

Turns out it was my homepod mini (hub) which was causing the problem...

Hey @bangelischan , i am curious, how did the homepod caused the problem?

And something I noticed:
My shortcut failed with the error code :
HMErrorDomain error 3.
(When I activate sleep mode; it was suppose to put the air purifier in sleep mode)

Le 6 juill. 2022 à 06:29, Bô Angélischan @.***> a écrit :

It acts as a hub for all my homekit devices, and for some reason, after rebooting it, everything works again.
That being said, i wake up this morning, and again, my levoit air purifier is still not responding…

Le 6 juill. 2022 à 01:52, Nicolae-Rares Ailincai @.***> a écrit :

Hey @bangelischan , i am curious, how did the homepod caused the problem?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.

I don't know why is not working, maybe some DNS, router or homepod isssues


[Idea] direct Communication between razor and js

if you see dotnet aspnet core issue
for creating more developer friendly features in razor language
we can create a way to use browser apis directly in .net without using jsInterop directly in razor file
and we can create new features and concept to avoiding user invalid names and...
for example
we have 1 razor component name Counter:

Counter.razor
Counter.razor.cs
Counter.razor.jsr
in Counter.razor user write Html like tags and attribute for razor page
in counter.razor.cs user write c# codes
and in Counter.razor.jsr user can write js codes that can called .net modules and in C# class we can call modules existing in this file.
for example contents must be like it:

counter.razor:

              Counter Page.

result is : ${Number}

          <button @onclick="Add"></button>

Counter.razor.cs:
public partial class Counter{

          public async Task Add(){
                 AddJsModule();
          }

}

Counter.razor.jsr
export Number
export AddJsModule(){

Number+=1;

}

this is very simple way to communicate between js razor and html and user can read and write easier than before

in this scenario we can create a jsInterop codes with SourceGenerator at compile time for each items in jsr file

@luohuarain
Yeah that more like c# and we can use that

@danroth27 what is your opinion?

Not sure I completely follow. Even with a new language, the underlying exposition for marshalling js + dotnet types is always going to be the JSImport and JSExport style workings. Is there some scenario where those are not right? Are you look for more compile time validation of that?

Yeah
That js export and import can be more easier to use and easier to integrating with dom and js libraries
For example that can be difficult to integrating with various js libraries like chat.js and etc...
If i wana to use chart.js in my page that i need create container in my page then create js modules in another file in wwwroot then import them in my razor page then call thats here developer might be make mistake in that codes and wirte names invalid or...
Then execute page check console logs then back to visual studio check codes then solve problems and boom....
Another problem happen 😂😂
Here we can reduceing developer mistakes in compile time and create trusted way for developers their can be sure js modules called true and theirs available and this is very easy to use and their no needed to code for import and export and etc...
For here ts can be good choice


Project Rating

Looking at the warehouse interface shuold be posible to show some rating, maybe a stars based system.

Dupe of #991 (double post).

Closing this as a duplicate.


who and how to use QPL library?

I means which application or library will use QPL library. is there some software stack to describe the position of the QPL.

Hi @Pinpiew, thank you for you interest in Intel® Query Processing Library!
Intel® QPL is a new library that basically provides APIs to communicate with Intel® In-Memory Analytics Accelerator (Intel® IAA). If you want more details on the architecture of the Intel® IAA, you could find the full specification on intel.com.
Essentially applications with a huge volume of data would benefit from using this library. The good example here would be Database managements systems. For instance, you could find the initial integration of Intel® QPL that has been recently added to ClickHouse®
Please, let me know if you have any other questions regarding the library or its usage.
Best,
Maria

I see, and no any other questions. thanks!


feat: range (f ∘ e) = range f

Match https://github.com/leanprover-community/mathlib/pull/18190

Please fix Type* and add an #align. Please merge roughly around the same time as the mathlib3 PR merges.
bors d+

I included these changes to Data.Set.Image in #2000 which forward-ports a newer change to this file. I'd suggest to close this PR and merge #2000

#2000 has to wait on https://github.com/leanprover-community/mathlib/pull/18491, so let's merge this first.
bors merge


Bug 2048370: CSI-Addons controller makes node reclaimspace request even when the PVC is not mounted to any pod

Node reclaimspace request should not be made
when nodeID is empty indicating the pvc is
not mounted to any pod and no volumeattachment
object for that PVC was found.
Signed-off-by: Rakshith R<EMAIL_ADDRESS>(cherry picked from commit 110c3a5a20af2c7d321a948536c741f1e3a6368a)

@Rakshith-R do you have required ack's? if not let us know once acks are in place we can approve it.. till you get ack place this PR on hold to avoid accidental merge.

@Rakshith-R do you have required ack's? if not let us know once acks are in place we can approve it.. till you get ack place this PR on hold to avoid accidental merge.

yes, I've asked for QE ack
:+1:
/hold

/bugzilla refresh

/unhold
/cc @Madhu-1

/lgtm


Add hud_panels=off to very low preset

It's intentionally not set. We used to set this, and we got so many support requests about how to re-enable them that it's clear we shouldn't set it.


Return the consequent if the test is truthy.

I would have written a test for this, but there didn't seem to be any tests and I wasn't sure whether you actually wanted any since this is experimental.

Yeah, I haven't written any tests since all the minification transformers are currently undocumented because I'm still trying to work out how it'd all work. Thanks!


Add pod disruption budget to the helm chart

This is a common practice for Kubernetes databases. We can define a pod disruption budget to minimize the cluster downtime (in combination with replication)

Hi @DengYiping thanks for your feedback, yes we are gonna work on adding this soon!


Ban of .com and .ps1 uploads

It seems that COM files can still be run in Windows, and they behave like any other executable.
PS1 files are scripts, so I put them on the list.

Okay, so turns out that whichever Android GitHub client I used adds "Downloading" to file name...
I'll fix that once I'm not on the train.

Fixed.

Iirc .ps1 files can't be executes unless you explicitly run a command to do so, so I think you should remove them, as well as .jars. The point of blocking certain file extensions is to avoid possible security risks, but we're not gonna hand-hold every clueless user. Don't download stuff you don't know how it works. Com and Scr are fine.

Well, .sh also need to be run via console.
Executables and scripts in general are a problem for users, but even more so for people who host the upload services. Someone downloads a virus and it encrypts their machine? Next time they'll be more careful. But it's host's site that's getting marked as malware site.
Those settings are meant as guidelines for hosts, from what I gather. If someone wants to enable any of those, they just need to delete a line while they deploy.

ps1's are linked to powershell in explorer and will be executed in powershell upon being clicked on


Better handling of PCF templates

They all show up in ControlTemplates.json. Shard DynamicControlDefinitionJson into pkgs?

Fixed with https://github.com/microsoft/PowerApps-Language-Tooling/pull/256


How to use Laravel Cashier stripe and braintree use for one model.

I am using laravel cashier for stripe and braintree payments .
So i migrated some fields for stripe and braintree for rooms table and add billable in the rooms model ..
When i execute it with stripe , It is working well ..
But braintree  is not working ..
I am getting "No such token: tokencc_bf_sdffcz_3d8ndc_dtqjzh_8gsc7f_zdz" this errors.
Rooms model
use Laravel\Cashier\Billable;

class Rooms extends Model
{
	use Billable
}

in service.php , I added this lines .
'stripe' => [
        'model'  => App\Models\Rooms::class,
        'key' => env('STRIPE_KEY'),
        'secret' => env('STRIPE_SECRET'),
    ],
    'braintree' => [
        'model'  => App\Models\Rooms::class,
        'environment' => env('BRAINTREE_ENV'),
        'merchant_id' => env('BRAINTREE_MERCHANT_ID'),
        'public_key' => env('BRAINTREE_PUBLIC_KEY'),
        'private_key' => env('BRAINTREE_PRIVATE_KEY'),
    ],

But btraintree is not working well
@taylorotwell
Do you have any suggestions for this ?
How can i fix this errors ?

Heya. Stripe and Braintree are two separate libraries. You cannot use them together at the same time. Either use one or the other.
https://github.com/laravel/cashier-braintree


[tests] Added TestBasicUsersIntegration

Added TestBasicUsersIntegration, which we can use in openwisp-radius to perform a basic integration.
Related to https://github.com/openwisp/openwisp-radius/issues/1

Coverage remained the same at 100.0% when pulling d3798ed6f058af628e556d7b0d7cafd25d21ad46 on qa into 9b287000dd5ce6f90ee44cdcd3ad1d9348022a87 on master.

Coverage remained the same at 100.0% when pulling d3798ed6f058af628e556d7b0d7cafd25d21ad46 on qa into 9b287000dd5ce6f90ee44cdcd3ad1d9348022a87 on master.


Add config option 'imagePullSecret' to the installation options file

This feature was added last year and is part of the Helm charts ( istio/istio@7759947) but this option never got included in the installation options file. This PR is to fix the issue reported in this original PR: https://github.com/istio/istio.io/pull/4446

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: johnma14
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: ostromart
If they are not already assigned, you can assign the PR to them by writing /assign @ostromart in a comment when ready.
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

install/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@johnma14: The following test failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/integ-istioctl-local-presubmit-tests.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test integ-istioctl-local-presubmit-tests-master

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@johnma14: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/integ-istioctl-local-presubmit-tests.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test integ-istioctl-local-presubmit-tests-master

prow/e2e-mixer-no_auth.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test e2e-mixer-no_auth-master

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@johnma14: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/integ-istioctl-local-presubmit-tests.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test integ-istioctl-local-presubmit-tests-master

prow/e2e-mixer-no_auth.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test e2e-mixer-no_auth-master

prow/release-test.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test release-test-master

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@johnma14: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/integ-istioctl-local-presubmit-tests.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test integ-istioctl-local-presubmit-tests-master

prow/e2e-mixer-no_auth.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test e2e-mixer-no_auth-master

prow/release-test.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test release-test-master

prow/integ-mixer-k8s-presubmit-tests.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test integ-mixer-k8s-presubmit-tests-master

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@johnma14: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/integ-istioctl-local-presubmit-tests.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test integ-istioctl-local-presubmit-tests-master

prow/e2e-mixer-no_auth.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test e2e-mixer-no_auth-master

prow/release-test.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test release-test-master

prow/integ-mixer-k8s-presubmit-tests.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test integ-mixer-k8s-presubmit-tests-master

prow/e2e-bookInfoTests-v1alpha3.sh
aff645d207d7165c5604d7528ee76cc046ce88d3
link
/test e2e-bookInfoTests-envoyv2-v1alpha3-master

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@howardjohn it wasn't included in the installation options file because it did not have a value. So for example, consider these examples:

global:
imagePullSecret:
- foo1
- foo2

global:
imagePullSecret:

In 1), global does not have a value since it is part of a nested element, whereas imagePullSecret has values. So the way the generator script works is, it needs to filter this so that we correctly
get global.imagePullSecret = ['foo1', 'foo2']
In 2), global and imagePullSecret both have no value, so it is ignored (which is what was happening with this value in the values.yaml file). Instead, if it is empty we should either use an empty string "" or empty list or dictionary.
Regarding the change in description, @brian-avery wanted an updated description. Hope this helps.

@johnma14: The following test failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/integ-security-k8s-presubmit-tests.sh
522732b719bdfb2f7e5d93dc34c0124423c5577b
link
/test integ-security-k8s-presubmit-tests-master

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: johnma14, sdake
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

install/OWNERS [sdake]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Why wasn't it included before? It seems like it already had a comment?

good question, perhaps the docification code doesn't handle multiple line comments?  If that is the case, it should.  Still this docification is easier for mere humans to understand :)

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: johnma14, linsun, sdake
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

install/OWNERS [linsun,sdake]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

New changes are detected. LGTM label has been removed.

@johnma14: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/integ-security-k8s-presubmit-tests.sh
522732b719bdfb2f7e5d93dc34c0124423c5577b
link
/test integ-security-k8s-presubmit-tests-master

prow/integ-mixer-k8s-presubmit-tests.sh
b613a83f20eff60611ccb0924c9366b200bd6864
link
/test integ-mixer-k8s-presubmit-tests-master

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

All authors explicitly sent this work, cla bot is just not very smart... Setting to yes

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: howardjohn, johnma14, linsun, sdake
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

install/OWNERS [howardjohn,linsun,sdake]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@howardjohn it wasn't included in the installation options file because it did not have a value. So for example, consider these examples:

global:
imagePullSecret:

foo1
foo2

global:
imagePullSecret:

In 1), global does not have a value since it is part of a nested element, whereas imagePullSecret has values. So the way the generator script works is, it needs to filter this so that we correctly
get global.imagePullSecret = ['foo1', 'foo2']
In 2), global and imagePullSecret both have no value, so it is ignored (which is what was happening with this value in the values.yaml file). Instead, if it is empty we should either use an empty string "" or empty list or dictionary.
Regarding the change in description, @brian-avery wanted an updated description. Hope this helps.

I'm not sure using an empty list or dictionary or string will work for all cases.  The installation code assumes an empty list is not the determining factor, but whether the value is defined at all (I think, but am not 100%).
It would be far better to make the documentation parser parse these comments and produce correct output when not key/value pair is found.

@sdake Key value pairs without a value get ignored at this line: https://github.com/istio/istio.io/blob/master/scripts/tablegen.py#L242. So the description gets processed only for the key-value pair that makes it into the prdict dictionary. I will see if there is a way I can modify the code to correctly identify valid empty key-value pairs.


Update OracleCLOBValueHandler.java

2000 is safer in case there are multibyte characters. interpolates just twice as many ')||TO_CLOB(' in worst case scenario.

Good point 👍


encoding problem with precompiled kotlin script plugins

I just spent hours to find out why my build suddenly throws strange errors. :-(
I'm working on the build for a library and was about to add maven publishing.
All worked well, until I suddenly got this error:
> Task :buildSrc:compileKotlin FAILED
e: ...\buildSrc\src\main\kotlin\net\kautler\publishing.gradle.kts: (4, 1): Expression 'publishing' cannot be invoked as a function. The function 'invoke()' is not found
e: ...\buildSrc\src\main\kotlin\net\kautler\publishing.gradle.kts: (4, 1): Unresolved reference. None of the following candidates is applicable because of receiver type mismatch:
internal val OrgGradlePluginGroup.publishing: PluginDependencySpec defined in gradle.kotlin.dsl.plugins._0f7525d1a3945796e226e609a50f6d81 in file PluginSpecBuilders.kt
public val PluginDependenciesSpec.publishing: PluginDependencySpec defined in org.gradle.kotlin.dsl

Which made absolutely no sense to me as I just added some comments.
I thought some cache is corrupt or whatever and tried all from completely cleaning everything including 15 GiB Gradle caches and so on, but the error persisted.
Finally after long trial & error and guessing, I boiled it down to this causing the issue:
package net.kautler

plugins { `maven-publish` }
publishing { }
// Björn

It is the ö in the comment which produces this strange error.
I tried to save the file as UTF-8, UTF-16, ISO-8859-1, all produced the same error.
The main problem is, I need this ö actually in the real code, not only in a comment, so just leaving it out is not an option.
So besides that maybe there could be some more meaningful option shown if possible and it should probably work properly, is there any way to work around this properly?

One workaround, once you discovered you have an encoding problem of course is to use unicode escapes instead of the problematic characters, so e. g. "Bj\u00f6rn" instead of Björn.

Thanks for the report @Vampire
Could you please tell us what Gradle version are you using? maybe more details on the host system and locale settings could also help down the road. A build scan would also help.
The thing is I can't reproduce the issue locally. A reproducer build would be great

5.4.1, Windows 10, de_DE.ISO-8859-1
I hopefully soon release this project to the OSS wild, then you can use it as reproducer by simply replacing the unicode escape back to an ö.
When kicking off the main build a build scan will not pre produced it seems as the  buildSrc compileKotlin is failing. I kicked off the buildScan build directly.
Buildscan with ö failing: https://scans.gradle.com/s/wekme65j2rpqm
Same build with the unicode escape: https://scans.gradle.com/s/q22cnd4tf7kc2

Grml I fell the same trap again.
I now added license headers to the files that of course also contain my name, and boom, all static accessors are unavailable while only the :buildSrc:compileKotlin step fails, the :buildSrc:generatePrecompiledScriptPluginAccessors step had no visual problem.

I would investigate it if you can't reproduce, but I have no real idea where to look at.

Something between 5.6.4 and 6.0 seems to have fixed this.
Leaving this open in case you want to investigate what caused this or want to add some test, but at least it works properly with Gradle 6 now.

Closing as I guess you don't want to investigate or test further


[v1.4] Create 'attending' command

List all events that current user is attending

Duplicate issue with https://github.com/CS2113-AY1819S1-T12-1/main/issues/108


fixes imdb data

noticed this wasn't returning any imdb datapoints, implemented a fix for ya

thanks a lot!
i just published v2.0.1 which contains your changes. changes:

removed node < v4 support
removed the ee-class dependency, now using es2015 classes
updated all dependnecies


is support expire?

is support expire?

Hey, what exactly are you looking for? If you want the lock to expire after a given time, you can use the lock_timeout_seconds setting referenced in the documentation.


Fix spelling

Description
This PR fixes spelling

Types of changes

[ ] Breaking change (fix or feature that would cause existing functionality to change)
[ ] New feature (non-breaking change which adds functionality)
[ ] Bug fix (non-breaking change which fixes an issue)
[ ] Enhancement (improves an existing feature and functionality)
[X] Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale

[ ] Major
[X] Minor

Bug Severity

[ ] BLOCKER
[ ] Critical
[ ] Major
[ ] Minor
[X] Trivial

Screenshots (if appropriate):
How Has This Been Tested?

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan package

UI build: :heavy_check_mark:
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/7002 (QA-JID-20)

Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5046

testing because it changes marvin;
@blueorangutan test

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

Trillian test result (tid-5600)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42420 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7002-t5600-kvm-centos7.zip
Smoke tests completed. 105 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test
Result
Time (s)
Test File

test_08_upgrade_kubernetes_ha_cluster
Failure
572.50
test_kubernetes_clusters.py


Add loadMessagesLater flag to props

This makes it possible to render the messenger while messages are still being fetched.
Before if no messages were supplied, allLoaded was set to true assuming there are no earlier messages to be loaded. Now by setting loadMessagesLater flag the messenger won't make that assumption.

Thank you @alizbazar


Fix timeline pdoc

Add back timeline methods as a hack for pdoc.

Codecov Report

Merging #128 (0e7138c) into main (24dcb3d) will decrease coverage by 0.01%.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
- Coverage   96.55%   96.53%   -0.02%
==========================================
  Files          43       43
  Lines        2377     2395      +18
==========================================
+ Hits         2295     2312      +17
- Misses         82       83       +1

Impacted Files
Coverage Δ

ical/timeline.py
98.43% <94.73%> (-1.57%)
:arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more


Publish 7.2-20210730011420+0000

Publish 7.2-20210730011420+0000

@bot-gradle test and merge

OK, I've already triggered a build for you.

Pre-tested commit build failed.

@bot-gradle test and merge

OK, I've already triggered a build for you.


Add Babel support for optional chaining

Upgrades Babel packages and adds support for optional chaining syntax.

Some nitpicking, though. Can we modify commit message to be more descriptive, please?

Thanks, looks like something got clobbered together.

Split the commits apart :+1:.


Missing permissions to Android 14 (specific to Samsung phones in particular)

After quite a bit of troubleshooting on Android 14, I ended up with a solution which might be of use to others with similar problems.
On Android 14 either emulating or on real device, no notifications appear, when app is sent to the background.
Solved by requesting Notifications with:
npm install @capacitor/push-notifications
and calling a function at app startup like:
async function registerNotifications(){
  let permStatus = await Capacitor.Plugins.PushNotifications.checkPermissions();
 if (permStatus.receive === 'prompt') {
    permStatus = await Capacitor.Plugins.PushNotifications.requestPermissions();
 }
  if (permStatus.receive !== 'granted') {
    throw new Error('User denied permissions!');
  }
}

Furthermore - and perhaps more critical to the operation of this plugin - background locations would stop completely when app is sent into background or screen is turned of on Samsung Android 14 devices (Galaxy S23 etc.).
Solved by adding:
    <uses-permission android:name="android.permission.FOREGROUND_SERVICE_LOCATION"/>
to AndroidMainfest.xml

The permissions requirement is documented in the README:

On Android 13+, the app needs the POST_NOTIFICATIONS runtime permission to show the persistent notification informing the user that their location is being used in the background. You may need to request this permission from the user.

It should not be necessary to add <uses-permission android:name="android.permission.FOREGROUND_SERVICE_LOCATION"/> to the app's manifest, because it is already included in the plugin's manifest. Seeing as the app and plugins' manifests are merged to produce the final manifest, I suspect this is unnecessary. Can you please check?

The permissions requirement is documented in the README:

On Android 13+, the app needs the POST_NOTIFICATIONS runtime permission to show the persistent notification informing the user that their location is being used in the background. You may need to request this permission from the user.

You are absolutely right. It just wasn't clear to me, that it wasn't enough to add POST_NOTIFICATIONS to AndroidManifest.xml, or to provide Notification permissions during run-time, which was the case. My app had to actively request Notification Permissions - in this case using another plugin.

It should not be necessary to add <uses-permission android:name="android.permission.FOREGROUND_SERVICE_LOCATION"/> to the app's manifest, because it is already included in the plugin's manifest. Seeing as the app and plugins' manifests are merged to produce the final manifest, I suspect this is unnecessary. Can you please check?

Sorry. Perhaps I just know too little about how Capacitor merges and packages the app and permissions. But I have checked. Removing <uses-permission android:name="android.permission.FOREGROUND_SERVICE_LOCATION"/> makes background-geolocation stop while in the background; tested on several separate tested Samsung Galaxy models running Android 14. I does not appear to be a problem on earlier versions of Android (11-12). Also, on submitting for internal test on Google Play, the Foreground Service Location permission was not identified before I added it manually (which meant I had to then upload a YouTube video etc. explaining how the app makes use of foreground services).
I am not sure if this is a Capacitor manifest merging issue. I tried looking through various manifest-merger log files present in my build folder hierarchy, but lost track of what it was doing.
This is what I see in the plugin's manifest - the FOREGROUND_SERVICE_LOCATION is not explicitly mentioned, using <uses-permission android:name="android.permission.FOREGROUND_SERVICE_LOCATION"/> as far as I can tell.
<manifest xmlns:android="http://schemas.android.com/apk/res/android">
    <application>
        <service
            android:name="com.equimaps.capacitor_background_geolocation.BackgroundGeolocationService"
            android:enabled="true"
            android:exported="true"
            android:foregroundServiceType="location" />
    </application>

    <uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION" />
    <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />
    <uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
    <!-- Android SDK 33+ requires the POST_NOTIFICATIONS runtime permission to
    display the foreground service notification. -->
    <uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
    <uses-feature android:name="android.hardware.location.gps" />
</manifest>

Whoa! I did not notice that FOREGROUND_SERVICE_LOCATION was different from FOREGROUND_SERVICE. I will add that to the plugin's manifest so there will be no need to add it to the app's manifest.
You are right that it's not at all obvious how to request notification permissions. I will link to this issue from the README.

Fixed in v1.2.16, please test.


Fixed package.json main entry point

The wrong entry point was taken and therefore the package did not work in IE. Fixed this by changing the path of "main" in package.json to dist file.

I removed dist from gitignore to use it as fork. Please ignore the second commit 8638c4f.


Support importing without content pipeline.

Add support for runtime importing of the files without the need to build first with the content pipeline.

Currently all importing and processing is done through the MonoGame.Aseprite.ContentPipeline.dll library In order for importing and processing to be performed during runtime, users would need to reference that library in their game project as well as the MonoGame.Aseprite.dll library.
The problem here is that both of these contain .Aseprite in their respective root namespaces, and it could potentially be confusing having the .ContentPipeline namespace as part of a completely different assembly.
Would potentially need to rename the namespaces to resolve this, which would be a breaking change.

Importing without the MGCB Editor is now possible in MonoGame.Aseprite Version 4.0.0.  https://github.com/AristurtleDev/monogame-aseprite/releases/tag/version%2F4.0.0


BREAKING CHANGE: Returns both value, and a cancel fn.

Attempt at Issue #1

@deanius I created the PR here, since your changes look good! Do you mind writing a test for it? then we can merge it :)

I'm on it..


chore: upgrade arrow array version to 52

As title.

duplicated with https://github.com/arrow-udf/arrow-udf/pull/64


Upgrade the Flask version to fix Jinja2 breaking change

https://github.com/line/jnotebook_reader/issues/17

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

Thanks, @mars-martin-line. :wink:


Create an eco-basic and eco-base release file

It would be useful if ECO would be one of the ontologies to test the new "-base.owl" product, this would be all asserted axioms in ECO (i.e may have references to OBI but no actual OBI axioms)
Would help with issues like #168 where ECO unwittingly is the conduit for axioms from OBI that end up polluting GO as we discovered here: https://github.com/geneontology/minerva/issues/195
(GO and other ontologies are also guilty of this kind of injection, we'll import base.owl too...)
cc @balhoff
Ah "basic" file would also work for GO's purposes (i.e. no external ontology classes at all) but the base file would be most useful for compositional purposes.

Hi @cmungall ! Should the base product only include the simple is_a hierarchy, like the "simple" versions created by OORT? I'll get this done this week.

Hi @rctauber, the 'base' file would be like the form described here: https://docs.google.com/document/d/1eCo5C3aZ9kjhBu98-24c2FHZIHEeFV7I2TJ4Vd6qw6k/edit#heading=h.u7or56kflnm2
So it's all the axioms (not just hierarchy) created by the ECO project, but no inferences materialized, no axioms provided by imported ontologies, and no imports declarations. I added one for CL here: https://github.com/obophenotype/cell-ontology/pull/509/files

Thanks @balhoff - so for the "base" file, I'll remove the import declarations and annotate it. No reasoning, no other processing steps.
For the "basic" file, I'll remove any equivalent statements and anonymous subclass statements. Would it make sense to remove the 'assertion method's from the "basic" file as well? Thoughts @cmungall @mchibucos ?

Thanks!

We have eco-base and eco-basic. Please reopen if this requires more discussion.


Replace docker packages tlsconfig and proxy with new library docker/go-connections.

Those packages have been moved to a standalone library and will be
removed from docker as soon as other projects stop depending on them.
The new library is a drop in replacement for the docker packages:
https://github.com/docker/go-connections
Signed-off-by: David Calavera<EMAIL_ADDRESS>

cycled dependency :goberserk:


Ingress GLBC: Please provide a way to enable Cloud CDN

The GLBC Ingress Resource should provide a way to enable the Google CloudCDN property on managed L7 LBs.
The CloudCDN flag is a property of an backend that can be enabled using the gcloud command line tool afterwards but since it's not clear how the GLBC ingress interacts with that on service updates it's not really practical.

Completely agree, we need this too, this could totally be a annotation of some sort.

It seems to work fine if you just enable the CloudCDN property after the Ingress has been created by K8s, but I still fear that a future update to the Ingress Controller may disable the CloudCDN flag. It would hit my frontend servers pretty hard if the CDN were suddently gone.

Yes this would be awesome!

Yep, we just enabled this manually, but I too would hate to see that flag suddenly be removed by the ingress controller.

We've just decided to move the Ingress outside of k8s because of this (and other things, such as health checks failing for services with multiple ports), but still, this is a vital part of the integration with GCP.

Completely agree that the CDN config should be part k8s ingress. I think the same is applicable to other features like 'session-affinity'.

@jeroenvisser101, for health checks of services with multiple ports, did you try to use the 'readinessProbe' config? Not too sure of your full requirements though

@SrinivasChilveri see here for limitations

I would also see this feature implemented as an annotation. Enabling CloudCDN after the initial provisioning works but I'd love to have it all in one place and managed by k8s.

So, is there a way to enable Google CDN on whole nginx-ingress-controller?

/remove-lifecycle stale

This is done?
https://cloud.google.com/kubernetes-engine/docs/how-to/cdn-backendconfig
@rramkumar1

@manjotpahwa1 Yes, we can close this.

Thanks!
does this work:
/close

Is it possible to make this work with the nginx ingress controller?

@albertkang In order to use the CDN feature of GCP, you will need to use the ingress-gce controller.


No added useful information

This document may as well not exist - it provides NO useful information above what is available from browsing the source code.
FFS: Document extract is only useful when useful information has been added to get extracted.

Document Details
⚠ Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

ID: 4cc93970-33b4-6d42-93f5-17ecda242b65
Version Independent ID: eb2fb4dd-15c5-4a6a-6969-4c8c22279b2c
Content: BackupLongTermRetentionPolicyInner.RecoveryServicesBackupPolicyResourceId Property (Microsoft.Azure.Management.Sql.Fluent.Models) - Azure for .NET Developers
Content Source: xml/Microsoft.Azure.Management.Sql.Fluent.Models/BackupLongTermRetentionPolicyInner.xml
Service: sql-database
GitHub Login: @rloutlaw
Microsoft Alias: routlaw

This is a reference doc that auto-generated from swagger spec, should be improved from source that service team can add more info.


Node/v8 compiles using only one core

I didn't see a flag for this in your documentation, and both npm config  set jobs N and export JOBS=N seem to be ignored. Is there a way to use more cores to compile the static node binary?

Yeah, it should be one of the make flag options. Doesn't use npm's config
as its not through gyp.
On Thu, May 12, 2016, 11:49 BruceBerry<EMAIL_ADDRESS>wrote:

I didn't see a flag for this in your documentation, and both npm config
set jobs N and export JOBS=N seem to be ignored. Is there a way to use
more cores to compile the static node binary?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
https://github.com/jaredallard/nexe/issues/220

oh ok I see it now... it's just not supported in the binary.

how to add such flag?

I just patched it in in my local installation by editing node_modules/nexe/bin/nexe and adding nodeMakeArgs: ["-j", "4"] to the argument passed to nexe.compile.
If you want a more permanent solution, you can either a) make your own script that calls nexe.compile with the flag (bin/nexe is just a command line wrapper around it after all) or b) register the flag with the argument parser used in bin/nexe and then make a pull request :-)


Update CODEOWNERS paths: fix invalid paths

As part of ongoing work of enabling wildcard support for CODEOWNERS:

https://github.com/Azure/azure-sdk-tools/issues/2770
https://github.com/Azure/azure-sdk-tools/pull/5088

and enabling stricter validation:

https://github.com/Azure/azure-sdk-tools/issues/4859

this PR:

fixes invalid paths, to match rules explained here;
removes /**/tests.yml and /**/ci.yml, to avoid all build failure notifications being routed to it once we enable the new regex-based, wildcard-supporting CODEOWNERS matcher, per: https://github.com/Azure/azure-sdk-tools/pull/5088#issuecomment-1397330147

As a consequence of this PR the automatically assigned PR reviewers will change for some paths. The full list of of such changes can be determined by running relevant test provided by this PR:

https://github.com/Azure/azure-sdk-tools/pull/5266

You can also reach out to me to get that information.
Once this PR is merged, I will enable the new CODEOWNERS matcher, similar to how it was done for net repo by these two PRs:

https://github.com/Azure/azure-sdk-tools/pull/5241
https://github.com/Azure/azure-sdk-tools/pull/5240

Doing so will also change to whom build failure notification emails are sent.
Related PRs:

Similar PR fixing invalid paths, but for net repo: https://github.com/Azure/azure-sdk-for-net/pull/33584
Similar PR deprioritizing the Azure SDK EngSys team ownership, but for python repo: https://github.com/Azure/azure-sdk-for-python/pull/28534

/check-enforcer override


chore(cjs/esm): Bundle module and use package exports

THIS IS A PROPOSAL
As explained in #1226, the fact that node-fetch v3.0.0-beta.10 is now pure ESM cause multiple problems with existing codebases.
In JavaScript codebases, we need to use the async import() which can create a lot of changes.
In TypeScript codebases (with cjs module mode), it is impossible to use node-fetch anymore.
I think that ESM is the future but it is not as stable as we think and it is not ready to be used everywhere.
The best way to ensure that all existing codebases can migrate to v3 is to provide both cjs and esm versions of this module.
For this, I introduced esbuild which can transpile ESM to CJS and also can bundle the package.
You can say: "node does not need packages to be bundled", which is true.
However, bundling is a good practice to reduce the module loading time, disk operations, memory footprint, etc.
It can also be used to bundle internal dependencies and so reduce the npm dependency tree (reduce install time, disk operations, memory footprint, npm hell, etc.).
What is the purpose of this pull request?

[ ] Documentation update
[X] Bug fix
[ ] New feature
[X] Other, please explain:

allow both esm and cjs exports
reduce module loading time
reduce disk operations
reduce module memory footprint

What changes did you make? (provide an overview)

use esbuild to bundle module in esm and cjs format
bundle dependencies to reduce module size
use package exports to automatically select the appropriate version
add build step to prepublishOnly
add build step to ci

Which issue (if any) does this pull request address?
#1226
#1217
Is there anything you'd like reviewers to know?
In this implementation, I bundle both data-uri-to-buffer and fetch-blob into the final result allowing us to be dependency free.
I think data-uri-to-buffer is safe to bundle because it's an internal dependency.
However, I think fetch-blob should not be bundled because it could be used by the consumer.
What do you think?
Note 1: If we do not want to bundle fetch-blob, we have to provide a CJS export for it.
Note 2: If we do not bundle fetch-blob, the install size will be very large as mentioned by @akaupi in #1217. Maybe we could make fetch-blob dependency free by using esbuild to bundle web-streams-polyfill .

I added a similar PR on fetch-blob (node-fetch/fetch-blob#107). It allows us to avoid bundling fetch-blob when using esbuild.
If and when merged, we could avoid bundling fetch-blob by changing package.json as follows:
diff --git a/package.json b/package.json
index 7f4726d..668aecf 100644
--- a/package.json
+++ b/package.json
@@ -21,8 +21,8 @@
   },
   "scripts": {
     "build": "npm run build:esm && npm run build:cjs",
-    "build:esm": "esbuild src/index.js --bundle --platform=node --format=esm --outfile=dist/index.mjs",
-    "build:cjs": "esbuild src/index.js --bundle --platform=node --format=cjs --outfile=dist/index.cjs",
+    "build:esm": "esbuild src/index.js --bundle --platform=node --format=esm --outfile=dist/index.mjs --external:fetch-blob",
+    "build:cjs": "esbuild src/index.js --bundle --platform=node --format=cjs --outfile=dist/index.cjs --external:fetch-blob",
     "test": "mocha",
     "coverage": "c8 report --reporter=text-lcov | coveralls",
     "test-types": "tsd",
@@ -66,7 +66,6 @@
     "data-uri-to-buffer": "^3.0.1",
     "delay": "^5.0.0",
     "esbuild": "^0.12.16",
-    "fetch-blob": "^3.1.2",
     "form-data": "^4.0.0",
     "formdata-node": "^3.5.4",
     "mocha": "^8.3.2",
@@ -74,6 +73,9 @@
     "tsd": "^0.14.0",
     "xo": "^0.39.1"
   },
+  "dependencies": {
+    "fetch-blob": "^3.1.2"
+  },
   "tsd": {
     "cwd": "@types",
     "compilerOptions": {

It feels very unappealing to basically double the hole codebase and start depending on a bundler just to compile everything to cjs and going back. using a module wrapper isn't a good solution either as other dependencies are esm only packages.
Many modules steers away from cjs nowdays.
cjs is considered as legacy and it's discouraged. new code should be using ESM instead. it makes it compatible with Deno and browser without having to use any bundler.
It's also quite unappealing to have a own version of whatwg:streams (coming from fetch-blob if we bundled everything and make it dependency free) as different version of whatwg:streams are not interchangeable with other versions. https://github.com/MattiasBuelens/web-streams-polyfill/issues/82#issuecomment-883672255
web streams polyfill could be used by other consumers as well.
I would rather suggest a very basic solution that goes something like:
exports = fetch = (...args) => import('index.esm').then(module => module.default(...args))

You won't be able to import Request, Response or Headers this way but not everyone needs those.

I think that publishing both CJS and ESM in the package is the wrong way to go. If it's super important to still support the old CommonJS way of importing the module, even though all currently supported Node.js versions supports ESM, I would rather see that we only published as CommonJS.
While there is a small upside in bundling the code before publishing to Npm, I think that we should also consider the downsides. E.g. one can no longer install a specific branch or commit by doing npm install node-fetch/node-fetch#aabbccd...

In this implementation, I bundle both data-uri-to-buffer and fetch-blob into the final result allowing us to be dependency free.

With this change we need to bump and publish a new package any time data-uri-to-buffer or fetch-blob publishes a new version. And if a security vulnerability is fixed in either of them, our users won't get that update until we publish a new version. I think that this is very bad, and I don't really see the upside. What is the benefit of doing this?

In JavaScript codebases, we need to use the async import() which can create a lot of changes.

Why can't you just do import fetch from 'node-fetch' as any normal dependency? 🤔

In TypeScript codebases (with cjs module mode), it is impossible to use node-fetch anymore.

This is easily solved by using "module": "ES2020" instead of "module": "CommonJS". Since every currently supported version of Node.js supports ESM, the ecosystem is moving to it. E.g. both me and Sindre is currently moving over all our packages and are only supporting ESM. And since ESM code can import CommonJS modules there is nothing stopping a project to move to ESM before all their dependencies are updated...

My opinion is that we should stick with ESM only without a bundler ☺

@LinusU

I think that publishing both CJS and ESM in the package is the wrong way to go.
Just a bit of curious, why do you think it's a wrong way?
I think conditional exports is born for it and else.

My opinion is that we should stick with ESM only without a bundler relaxed
Agree but not agree. As you can not write CJS and ESM at the same time.
You need to choose at least one from

transpiler
bundler

Transpiler: typescript
Pro: I think might be best option, can easily transpile to ESM, CJS, etc.
Con: Still not support output extension name with .mjs!!!
Transpiler: babel
Pro: ...
Con: need to maintain babelrc + its plugins
Bundler: rollup
Pro: ...
Con: need to maintain babelrc + its plugins
Bundler: esbuild
Pro: out-of-box, no config, no need plugins
Con: less easy extendable than rollup
So, as I understand why dev drop CJS.
I might on my own. To decrease maintainance, I choose esbuild.
But, I still want to say, please re-consider. Support both as we are still not there yet.

@loynoir i feel strongly against bundeling whatwg streams and having a own instance (of everything basically) that are not interchangeable just b/c of this reason alone:
https://github.com/MattiasBuelens/web-streams-polyfill/issues/82#issuecomment-883672255
Do you have any arguments against that?

Just a bit of curious, why do you think it's a wrong way?

I don't see any benefit of doing this, over just publishing a CommonJS module, since ESM can import CommonJS modules. If you know of any benefits I would be happy to hear them!
The downside is that you are shipping your entire module twice, which means (apart from doubling the size) that there is a risk that you'll load two different but very similar versions of the package at runtime. This can then lead to the "dual package hazard" which can cause runtime breakage. e.g. https://github.com/GeoffreyBooth/dual-package-hazard

Agree but not agree. As you can not write CJS and ESM at the same time.
You need to choose at least one from
[...]

I don't really understand this 🤔
Node.js supports importing ESM modules natively, you do not need neither a bundler or a transpiler?

Hi, @jimmywarting.

Do you have any arguments against that?

I do not want to argue with anyone, as I said I totally understand the movement. And I'm also a big fan of ESM.

i feel strongly against bundeling whatwg streams and having a own instance

Agree. But...😂
$ esbuild --format=cjs --outdir=dist/cjs ./src/*js ./src/errors/*js ./src/utils/*js
dist
└── cjs
    ├── body.js
    ├── errors
    │   ├── abort-error.js
    │   ├── base.js
    │   └── fetch-error.js
    ├── headers.js
    ├── index.js
    ├── request.js
    ├── response.js
    └── utils
        ├── form-data.js
        ├── get-search.js
        ├── is.js
        └── is-redirect.js

> require('./dist/cjs')
Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: fetch-blob/index.js

Just lack of energy to find out all ESM only dependencies, and fork every of them. 😂
I don't think bundler is a good idea, but at least it's acceptable and quick workaround, for me.
Hi, @LinusU

Node.js supports importing ESM modules natively, you do not need neither a bundler or a transpiler?

If I rememeber correctly, the last time I check jest source code, jest use vm, and node.js
vm ESM support is still experimental --experimental-vm-modules.
So, maybe it's not native enough.
Hi, @SomaticIT
Maybe let esbuild do no bundeling,
esbuild --format=cjs --outdir=dist/cjs ./src/*js ./src/errors/*js ./src/utils/*js

After fetch-blob is not ESM only.
> require('./dist/cjs')
Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: fetch-blob/index.js

Support both as we are still not there yet.

The stable 2.x line is still fully functional and this package is still in beta. I think it's reasonable that people upgrade to 3.x at the same time when they make sure that they are running a supported versions of Node.js and switches to ESM.

Well…I’d be happy to stick to 2.x if you’d backport some of the features in 3.x I’m using. You‘re effectively making the same argument here about maintaining multiple codebases if you intend to keep 2.x and 3.x (once it comes out of beta) going, no?
Also - I think one of the scenarios that’s being missed is situations like mine: some of my code that uses node-fetch are plugins to other projects that don’t yet support ESM. You’re forcing me to version lock in a situation I have little control over, as I’m operating in a broader ecosystem that’s slower to implement ESM support. Now my choices are:

Switch away from node-fetch. (not my preference)
Lock support to beta9 and hope that there’s no major bug or security implication that needs to be resolved in the future until I (and the broader ecosystem I’m writing plugins for) can all upgrade to ESM. (what I’ve opted to do for the time being).
Revert to 2.x and lose features that I rely on, and introduce other potential problems.

We all recognize it’s beta, and stuff happens and major breaking changes is part of “stuff”. I can make the argument both ways. :smile:

have you tried just doing:
const = fetch = (...args) => import('node-fetch').then(module => module.default(...args))

...to see if it works?

have you tried just doing:
const fetch = (...args) => import('node-fetch').then(module => module.default(...args))

...to see if it works?

I think it should work but if exported by node-fetch, you would have a different module exports (no more Headers, Request, Response, etc. classes exported).

@loynoir i feel strongly against bundeling whatwg streams and having a own instance (of everything basically) that are not interchangeable just b/c of this reason alone:
MattiasBuelens/web-streams-polyfill#82 (comment)
Do you have any arguments against that?

Bundling of dependencies is totally optionnal. I used it because, web-streams-polyfill is very big when installed because it contains polyfills for every runtime versions. I thought that we could use only the good one in our environment.

It feels very unappealing to basically double the hole codebase and start depending on a bundler just to compile everything to cjs and going back. using a module wrapper isn't a good solution either as other dependencies are esm only packages.
Many modules steers away from cjs nowdays.
cjs is considered as legacy and it's discouraged. new code should be using ESM instead. it makes it compatible with Deno and browser without having to use any bundler.
It's also quite unappealing to have a own version of whatwg:streams (coming from fetch-blob if we bundled everything and make it dependency free) as different version of whatwg:streams are not interchangeable with other versions. MattiasBuelens/web-streams-polyfill#82 (comment)
web streams polyfill could be used by other consumers as well.
I would rather suggest a very basic solution that goes something like:
// something.cjs
exports = fetch = (...args) => import('node-fetch').then(module => module.default(...args))

You won't be able to import Request, Response or Headers this way but not everyone needs those.
My opinion is that we should stick with ESM only without a bundler
If you wish to use node-fetch@v3 then you should be using ESM as well... all new code shouldIt is time that we steer away from using it, cjs won't go away unless we stop using it.
if you are stuck with cjs then better use any other ajax library, wait until fetch gets implemented into core or stay with v2.

There is no doubling of codebase, you keep the same source code for both exports.
Unlike what many people thinks, bundling is a very good practice for performance (even without external dependencies). The public package is tinier and it loads faster (less disk operations).
Having both CJS and ESM exports allows all existing consumers to keep their codebase without changing any line of code. I think that it's very important when you maintain a library which is very popular like node-fetch.
I'm also a big fan of ESM but I think that we should be concerned of existing codebases before make such a breaking change.

The stable 2.x line is still fully functional and this package is still in beta. I think it's reasonable that people upgrade to 3.x at the same time when they make sure that they are running a supported versions of Node.js and switches to ESM.

We should think of when v3 will be stable. You will have a lot of issues of people upgrading their dependencies and having trouble with their existing codebases.

Thanks for all you comments.
I can upgrade this PR to avoid bundling dependencies if you think it's preferable.

I think it should work but if exported by node-fetch, you would have a different module exports (no more Headers, Request, Response, etc. classes exported).

You can still load headers, req, res etc but it has to be done async. Another way if you still have a large codebase is to bootstrap your application to load node-fetch first and then run your own application
// main `./bootstrap.js` file in cjs format

// Load node-fetch first.
import('node-fetch').then(mod => {
  // Assign fetch to global namespace
  const {default: fetch, ...rest} = mod.default
  Object.assign(globalThis, rest, { fetch })

  // Run your main legacy application that is built entirely in commonjs format.
  import('./index.js')
})

I can upgrade this PR to avoid bundling dependencies if you think it's preferable.

That won't solve the fact that fetch-blob is a ESM only package. If we switch to cjs too then we can't load the fetch-blob cuz esm packages are loaded asynchronous by nature. You would have to either bundling everything or nothing.

One use case for cjs even in modern node environment is server apps build with webpack.
It is pretty common to compile react app for node to enable server-side rendering simply because jsx syntax is not native JS feature and webpack one of the most popular tools for that.
But webpack also transforms native imports to own implementation that uses require() under the hood. Which makes node app bundled with webpack incompatible with esm module imports.
Yes, it is possible to include node-fetch into bundle and transform it to cjs on app level in cases like this but:

it is not recommended in general for node target (external package might use binary files, etc.)
it is very unreliable because end-user need to analyse which esm dependencies node-fetch has and whitelist it as well from externals

I wish webpack can support esm compilation target so imports can be left untouched but it is still under development and does not seems to be released soon.

Originally posted by @gbiryukov in https://github.com/node-fetch/node-fetch/issues/1226#issuecomment-898407517

One use case for cjs even in modern node environment is server apps build with webpack.

That is a problem with webpack, not with node-fetch. Since every supported version of Node.js supports ESM modules, there is no point adding CommonJS bundles.

Indeed. The question is how many users of webpack will face this issue and spend their time to find workaround (which is not extremely simple due to couple of reasons).
Personally to me it seems like majority of the users of node-fetch are people with universal apps which means 90% chance of having webpack in the pipeline.
Cjs build or at least simple guide how to use node-fetch with webpack will eliminate a lot of friction

If people are building with webpack, will it not simply inline this entire module, which will eliminate the problem altogether?
Also, webpack has support for outputting a bundle that uses import/export instead of require

with webpack, will it not simply inline this entire module, which will eliminate the problem altogether?

when app bundled for target node it is good practice to do not bundle third parties because some of them can use/install binaries, can do something important for environment in port-install etc. There are also couple of reasons for library authors.

Also, webpack has support for outputting a bundle that uses import/export instead of require

true but this feature only available in experimental mode and only for v5 users which have pretty small adoption.
I understand intention to keep things simple and forget about cjs (and other not most up to date stuff) but just makes such a pain to use node-fetch for many people. It also seems to be source of many similar issues in this repo so at would be great at least to document how to use it in cjs build

Personally to me it seems like majority of the users of node-fetch are people with universal apps which means 90% chance of having webpack in the pipeline.

If you are using webpack to bundle stuff then it most likely mean frontend apps? in which case window.fetch should be used instead of node-fetch. (something like isomorphic-fetch)
Targeting Deno? then use Deno's global fetch api. It is a waste to include something that already is supported

least to document how to use it in cjs build

That is what we are trying to achieve with #1236

I've added some context above

It is pretty common to compile react app for node to enable server-side rendering simply because jsx syntax is not native JS feature and webpack one of the most popular tools for that.

So the need for node-fetch in webpack bundled node app is to mimic browser environment and make server-side rendering work same way as in the browser

I suppose you are talking about native react (for phones) and not the web version of react?

I am talking about web version of react. It uses own syntax (JSX) that is not part of ES6 so it needs to be transformed into ES6 before running in both browser and node environments

During this transformation process, Webpack would transpile node-fetch to CommonJS so that it can run in older browsers.

It's final - v3 got released as stable with ESM only support
After consideration we think this will be for the best in the long run
yea, it can be annoying at first to try and switch your own cjs package, to esm but it's only a transition period and we need to make the switch eventually.

This is a bad decision. ES modules are not ready for prime-time. And its not trivial to shift large projects to ES modules.
It just cuts off new bug fixes and features to non-ESM packages.
Take the expo project for example. xdl > @expo/dev-server > node-fetch. You are basically asking them to migrate the huge xdl codebase to ESM in order to get bug fixes.
And if you use any kind of import/require hooks, well the API for this is not stable and will change soon. So if you rely on these features, not only is it a lot of dev work, you simply cannot because ESM is not ready.
Same discussion here regarding Sindre's packages which probably kicked off this chaos.
Conditional exports was built into Node.js to avoid this pain...and every library just ignores it.

https://github.com/sindresorhus/meta/discussions/15#discussioncomment-1258548

FYI, AWS Lambda (and other platforms based on Lambda, e.g. Netlify functions) still doesn't have ESM support in its Node14 implementation. So anyone building Lambda functions will need to package their code as CJS. See https://stackoverflow.com/questions/66676555/nodejs-14-x-native-aws-lambda-import-export-support/66688901#66688901 for updates.
When I have time, I'm going to investigate using esbuild (probably via serverless-esbuild because my Lambda functions are built with the serverless framework) to transform ESM code into CJS so that AWS Lambda will run it. I've never used esbuild before so I'm not sure this will work, but it seems easy enough to try it.
Sadly, I suspect that the ESM-only approach that node-fetch and Sindre are doing is what will be needed to force the rest of the ecosystem (looking at you, AWS!) to get their own ESM support shipped.  It's like a big game of tech chicken, and hopefully ESM will win the game before everyone crashes! :-)

Another use case that brought me here is compiling to commonjs for unit tests only, to be able to spy on exported functions (see discussion in https://github.com/jasmine/jasmine/issues/1414#issuecomment-501209914 and https://jasmine.github.io/pages/faq.html#module-spy)

@justingrant I would recommend you to bundle your code before shipping to Lambda in any case, it makes a ton of difference (in my experience) with the size of the Lambda bundle, and the cold boot time. You can use e.g. ncc to do this without much setup at all!
For Lambda I also think it's very good that the ecosystem is aggressively moving to ESM, because without that I don't think that AWS will implement support for it anytime soon. Many packages not working without it will hopefully pressure them to fix ESM support (which by the way would be very easy for them, if you dump their startup code you can see that it basically is just to replace a require(...) with an import(...) call, since they are loading your code dynamically already).

I can't think they have ever worked on an even slightly large Node.js project with diverse dependency trees

we have...

Or realize that Webpack itself is still broken when trying to make your project fully ESM as output

That's webpacks problem. We considered packing node-fetch our self to have dual suport but the conns outweighed the pros

I have spent the past week of wasted time trying to resolve these require()

v3 have only been out for 2 days...

And its often not our choice at all- we might just update a single module for a security release- and that module has now made use of a new version of a module (node-fetch for example)

you often have a choice, you can stick with v2.x if you like, it's very stable, it has fixed many bugs and haven't been touched much
We will continue to backport and fix security issues with the version 2,

That's webpacks problem

This is the lack of empathy and understanding of the JavaScript ecosystem as a whole that I'm talking about. I don't think you realize just how many projects have Webpack as a core part of their packaging pipeline. This is why I'm saying its premature, there are options given to you to make this transition easier on everyone (such as pointed out by @vjpr, conditional exports)- but you have decided to take the "scorched-earth" policy instead.

v3 have only been out for 2 days...

node-fetch is a recent one that I have updated- but the biggest issues were most of @sindresorhus modules- which is literally thousands which are imported in all kinds of nooks and crannies of the JavaScript ecosystem. Including so many little "one-line" packages such as is-plain-obj etc.

you often have a choice, you can stick with v2.x if you like, it's very stable, it has fixed many bugs and haven't been touched much. We will continue to backport and fix security issues with the version 2,

While I appreciate that... as I said we often don't make that choice directly ... This is why I ask if you have ever really worked on large JS projects, possibly monorepos that link up and use 100s of diverse dependencies, some of which have decided to update a dependency to a version that uses ESM (unknowing to the library author how that might effect downstream users because there won't be any errors popping up when he bundles his library) which now causes all projects which update (sometimes even a minor version, because their API hasn't changed at all) to suddenly be faced with all kinds of errors. Now imagine that this kind of thing happens- but its 3 dependencies deep. This is a huge mess, and I don't think its fair on users who are simply trying to be productive to tell them to now chase down all these version numbers and fix everything, or make PRs to all his dependent projects to back-port needed functionality to the version that makes use of an older non-ESM dependency... especially when popular tooling, such as Webpack 5 (probably the most used JS bundling tool by far) still does not support outputting our projects to ESM to make this transition easier.

@Richienb

That is a problem with webpack, not with node-fetch. Since every supported version of Node.js supports ESM modules, there is no point adding CommonJS bundles.

Please. Please. Please.
Make a pull to Node.js, make --experimental-vm-modules from experimental to stable.
Jest is running under vm module.

Although I find a way out, but

The ESM tool ecosystem is not complete
ESM-only under vm context is experimental, --experimental-vm-modules & jest
I don't think it's a proper time for ESM-only.

@jimmywarting
Please image how many current hours and future hours, around the whole world used, within node-fetch dependency network, just because you decide to ESM-only over dual-module.
Please re-consider.

@ LinusU

I don't see any benefit of doing this, over just publishing a CommonJS module, since ESM can import CommonJS modules. If you know of any benefits I would be happy to hear them!

Actually there is one.
The full ESM-only ecosystem haven't grown up.
Just image how many time whole world downstream user will use, if popular upstream library decide to ESM-only while the whole ecosystem haven't ready.
One reason, but much. But human time are precious, which make one reason weight enough?

Node.js supports importing ESM modules natively

--experimental-vm-modules
esm_loaders: "This API is currently being redesigned and will still change."
I love and using ESM, but ESM is not totally ready. But it is not yet a ESM-only period.

Hello everyone,
Just some figures from npm:

21 449 dependant packages
21 653 327 weekly downloads

It's only npm. Based on the number of dependant packages and weekly downloads, you can estimate that there are millions of projects which uses node-fetch.
So this decision is impacting millions of developers around the world who will encounter an unexpected issue when migrating to node-fetch v3. They will have to spend multiple hours to understand the issue and find a workaround that works in their specific environment.

This decision will cost millions of hours (and dollars) in the javascript development ecosystem. You will directly impact the job of millions of developers and companies. Finally, all the time lost on node-fetch is not spent innovating.
This could become the worst decision in the node.js ecosystem in 2021!

And this decision is taken only because you don't want to introduce a transpiler (here esbuild).
In this PR, I suggest to not-only transpile the code but also bundle it (for performance). If you don't like bundling, we could only transpile the code. Transpilation is a safe process, it can be tested during the CI and it will not take you too much time.
I'm also a huge fan of ESM and my conviction is that it's the future of javascript. I think it should be used on every new projects. But here we talk about maintaining a globally used library. You can't just impose such a breaking change to millions of people because it's your (own) conviction. Please be humble and reconsider your convictions.
Note: Also think about TypeScript users who could not mix CJS require and ESM imports. They are just stuck, they will never be able to update node-fetch (no more bug fixes, security patches, etc.).

Edit: you should also think about TypeScript users who could not mix CJS require and ESM imports. They are just stuck, they will never be able to update node-fetch (no more bug fixes, security patches, etc.).

TS users already can't use it because they BROKE THE TYPES!! (#1262)

@jimmywarting @SomaticIT
Should the title to Re-add dual module support?  Add using bundler is just an approach to inline ESM only node-fetch/fetch-blob
If concern about release size, can also use postinstall field, right?
Plus, typo ems -> esm

the release size is not the only thing on my mind. it's about not having 2 instances of whatwg:streams or anything else for that mater
https://github.com/MattiasBuelens/web-streams-polyfill/issues/70#issuecomment-851058462
A ReadableStream created from version 2.1.1 can not pipe data to a WritableStream constructed by version 2.1.2
if we would have a dual model support with commonjs then it means that we need to reintroduce a bundler and package everything into one file and have a own instances of ReadableStream, a own instances of fetch-blob, base64 parser and every other dependency that we depend on.
that duplicates all code, it creates own instances of everything and it would be incompatible with a stream that you yourself are using.

@SomaticIT is there anything preventing you from staying on 2.x as long as you need CommonJS, and then upgrading to 3.x when you are switched over to ESM?
As was said before:

[...] you can stick with v2.x if you like, it's very stable, it has fixed many bugs and haven't been touched much
We will continue to backport and fix security issues with the version 2,

@jimmywarting
Not very understand. Is node-fetch facing problem ESM and CJS state references is not same?
Maybe just rename<EMAIL_ADDRESS>as new channel or something like node-fetch@next, get away from <PM> install node-fetch.
As it's is breaking update.
FYI, I'm using pnpm. I updated without see any warning.

@jimmywarting, There is no problem to stay with unbundled dependencies. We just need to set them as external when running esbuild.
@LinusU, Yes for 3 reasons:

v2 typings are not correct (I collaborated with you to build the v3 typings the right way).
I wonder if v2 will continue to be updated with bug fixes and security patches.
In my company, we have automated processes that ensures that all our dependencies stay up to date and block our CI pipelines if they are not. This is something we guarantee to our customers. Ignoring a library like node-fetch which communicates over the network is a real threat and could create a legal problem.

@LinusU, Moreover, I'm not the only one to considerate. As I said before, there are millions of projects which uses node-fetch and many of them (like some of mines) are big projects with 100k+ lines. It could take years for such projects to migrate to ESM.

in case of universal apps (that runs both in browser and in node) v3 is very desirable simply due to better compliance to the the spec so all features can be safely used like Response.error() method that is missing in v2 but available in v3

Hello everyone,
Just to let the community knows, after a lot of issues in my existing projects, I created a wrapper of node-fetch to use in commonjs environments.
The goal of this package is to help commonjs users who have difficulties to migrate their existing codebase to v3 while avoiding to increase the work for node-fetch authors.
You can find it here: touchifyapp/node-fetch-cjs.
If you are able to migrate to the official v3 release of node-fetch, I highly recommend to use the official node-fetch. This package is built to help users who could not migrate easily.
This repository is automatically updated when a new version of node-fetch is released

I'm wondering if anyone has any suggestions for if there is a way to use node-fetch v3 in my specific situation. I am currently on a project using webpack which outputs cjs because we are stuck using node 14 in an aws lambda. esm is only experimental in node14, so I can't switch the whole project over to esm. Just curious if there are any other options besides stay with v2 or switch to another library.

@melissarh57  ESM should be supported all the way from v12.17 and cjs should be able to load esm only packages using async import.
maybe this solution can work for you https://github.com/node-fetch/node-fetch/issues/1279#issuecomment-915063354 ?

https://nodejs.org/en/blog/announcements/v18-release-announce/#fetch-experimental

In Node.js 18, an experimental global fetch API is available by default.

$ node
Welcome to Node.js v18.0.0.
> fetch
[AsyncFunction: fetch]


The unmeaning Region parameter became mandatory for MRAP endpoint

Describe the bug
After an upgrade from 1.9.157 to 1.11.4, the parameter Region became mandatory when trying to connect to a MRAP accesspoint.
As described in documentation https://docs.aws.amazon.com/AmazonS3/latest/userguide/MultiRegionAccessPointRequests.html

Multi-Region Access Point ARNs don't include an AWS Region

The region should not be mandatory when using MRAP accesspoints.
It should either be ignored or a warning should be raised/logged.
Expected Behavior
The S3 requests are successful without the need to set a region ; the same as it was with SDK 1.9.157.
Current Behavior
Below are the traces that are logged when we create the S3Client() class and create the HeadObject request.
[DEBUG] 2023-04-24 13:49:14.401 ClientConfiguration [140256500507328] Request Compression enabled
[DEBUG] 2023-04-24 13:49:14.401 ClientConfiguration [140256500507328] ClientConfiguration will use MinReqCompression: 10240
[DEBUG] 2023-04-24 13:49:14.401 ClientConfiguration [140256500507328] ClientConfiguration will use SDK Auto Resolved profile: [default] if not specified by users.
[WARN] 2023-04-24 13:49:14.401 ClientConfiguration [140256500507328] Retry Strategy will use the default max attempts.
[INFO] 2023-04-24 13:49:14.401 CurlHandleContainer [140256500507328] Initializing CurlHandleContainer with size 25
[TRACE] 2023-04-24 13:49:14.403 Aws::Endpoint::DefaultEndpointProvider [140256378881600] Endpoint bool eval parameter: UseFIPS = 0
[TRACE] 2023-04-24 13:49:14.403 Aws::Endpoint::DefaultEndpointProvider [140256378881600] Endpoint bool eval parameter: UseDualStack = 0
[TRACE] 2023-04-24 13:49:14.403 Aws::Endpoint::DefaultEndpointProvider [140256378881600] Endpoint bool eval parameter: UseArnRegion = 0
[TRACE] 2023-04-24 13:49:14.403 Aws::Endpoint::DefaultEndpointProvider [140256378881600] Endpoint bool eval parameter: DisableMultiRegionAccessPoints = 0
[TRACE] 2023-04-24 13:49:14.403 Aws::Endpoint::DefaultEndpointProvider [140256378881600] Endpoint str eval parameter: Bucket = arn:aws:s3::123456789012:accesspoint/mfzwi23gnjvgw.mrap
[ERROR] 2023-04-24 13:49:14.403 HeadObject [140256378881600] A region must be set when sending requests to S3.

Workaround with Region=invalid-region:
[DEBUG] 2023-04-24 14:16:37.120 ClientConfiguration [140069699732160] Request Compression enabled
[DEBUG] 2023-04-24 14:16:37.120 ClientConfiguration [140069699732160] ClientConfiguration will use MinReqCompression: 10240
[DEBUG] 2023-04-24 14:16:37.120 ClientConfiguration [140069699732160] ClientConfiguration will use SDK Auto Resolved profile: [default] if not specified by users.
[WARN] 2023-04-24 14:16:37.120 ClientConfiguration [140069699732160] Retry Strategy will use the default max attempts.
[INFO] 2023-04-24 14:16:37.120 CurlHandleContainer [140069699732160] Initializing CurlHandleContainer with size 25
[TRACE] 2023-04-24 14:16:37.121 Aws::Endpoint::DefaultEndpointProvider [140069578106432] Endpoint str eval parameter: Region = invalid-region
[TRACE] 2023-04-24 14:16:37.121 Aws::Endpoint::DefaultEndpointProvider [140069578106432] Endpoint bool eval parameter: UseFIPS = 0
[TRACE] 2023-04-24 14:16:37.121 Aws::Endpoint::DefaultEndpointProvider [140069578106432] Endpoint bool eval parameter: UseDualStack = 0
[TRACE] 2023-04-24 14:16:37.121 Aws::Endpoint::DefaultEndpointProvider [140069578106432] Endpoint bool eval parameter: UseArnRegion = 0
[TRACE] 2023-04-24 14:16:37.121 Aws::Endpoint::DefaultEndpointProvider [140069578106432] Endpoint bool eval parameter: DisableMultiRegionAccessPoints = 0
[TRACE] 2023-04-24 14:16:37.121 Aws::Endpoint::DefaultEndpointProvider [140069578106432] Endpoint str eval parameter: Bucket = arn:aws:s3::123456789012:accesspoint/mfzwi23gnjvgw.mrap
[DEBUG] 2023-04-24 14:16:37.121 Aws::Endpoint::DefaultEndpointProvider [140069578106432] Endpoint rules engine evaluated the endpoint: https://mfzwi23gnjvgw.mrap.accesspoint.s3-global.amazonaws.com
[TRACE] 2023-04-24 14:16:37.121 Aws::Endpoint::DefaultEndpointProvider [140069578106432] Endpoint rules evaluated props: {"authSchemes":[{"disableDoubleEncoding":true,"name":"sigv4a","signingName":"s3","signingRegionSet":["*"]}]}
[TRACE] 2023-04-24 14:16:37.121 AWSClient [140069578106432] No content body, content-length headers
[DEBUG] 2023-04-24 14:16:37.121 AWSAuthSymmetricV4Signer [140069578106432] Note: Http payloads are not being signed. signPayloads=0 http scheme=https
[DEBUG] 2023-04-24 14:16:37.122 AWSClient [140069578106432] Request Successfully signed
[TRACE] 2023-04-24 14:16:37.122 CurlHttpClient [140069578106432] Making request to https://mfzwi23gnjvgw.mrap.accesspoint.s3-global.amazonaws.com/**
...

Reproduction Steps
In aws-sdk-cpp/generated/tests/s3-gen-tests/S3EndpointProviderTests.cpp, remove EpParam("Region", "us-east-1") for any mrap test case.
Possible Solution
Rework the file aws-sdk-cpp/tools/code-generation/endpoints/s3-2006-03-01.endpoint-rule-set.json o that MRAP arn are correctly handled.
Additional Information/Context
Workaround is to set any (even invalid) text value in Region parameter.
AWS CPP SDK version used
1.11.64
Compiler and Version used
gcc 11.3
Operating System and version
Linux 5.15

Hi @gab-david ,
I'm really sorry you are facing this new limitation. Thank you a lot for diving deep into the implementation details of how it works now.
The current AWS SDKs approach is to consider that the Region config parameter is always present.
And we (as the SDK), don't own the endpoint rules file. We have a workaround for legacy configs of overriden endpoint, where we set a fake region, however for the MRAP use case, it is hard to guess if user is going to make MRAP calls at the client construction time.
Long story short, your workaround is valid and I would suggest to keep it.
I will check internally if SDKs could relax the relatively new limitation of a required region, even for MRAP, but I'm afraid it won't be changed.
Best regards,
Sergey

Hi @SergeyRyabinin,
Thanks for your quick answer.
I also see that the region is used to set the partition in rules file and then aws_map_region_to_partition() is called.
Is (or will be) MRAP supported for these partitions ? Like in China or for aws-us-gov even if they don't appear in the list of 'Only the following AWS Regions are supported' https://docs.aws.amazon.com/AmazonS3/latest/userguide/MultiRegionAccessPointRestrictions.html
For this issue, we'll use the workaround with the same fake region.
Best regards,
Gabriel

Hi @gab-david ,
As you pointed out, region is used to determine the partition.
However, our current API does not provide an explicit way of selecting a partition.
When/if the SDK is going to provide such support, we will have to introduce an additional API parameter or figure out the correct partition from the ARN, request object. Sorry, I cannot comment / provide guidance on this topic at this moment.
Best regards,
Sergey


WIP: transition to zwavejs

There will be many breaking changes but that's also an occasion to make something better. About the docs-related reviews I haven't worked so much on it so there will be many things missing or that need edits

TODOs:

[ ] Fix hass discovery: everything is using valueId index, need to support the new property/propertyKey, also some valueIds use different topics for read/write (like currentValue and targetValue)
[ ] Docs: general docs fix
[ ] Scenes
[ ] Zwave events
[x] Basic MQTT Gateway and Control Panel functions

@chilicheech Unfortunally I don't own many zwave devices to make tests with hass. Could you help me to work on the hass support?
Essentially what is needed is to translate all valueId.index references in Gateway.js to use property and propertyKey. Also some command classes now have two separated valueIds with read only and write only values, one is used to write and one to read, so the state needs to be set to the read only value and the set topic to the write only value. Also rgb some values (like rgb) now are splitted in  separeted values (r, g, b) each one has a different propertyKey, for this values we should need to create another topic another propertyKey like rgb where the value could be in the format required by hass.

I suggest to merge this so at leat there will be also a docker dev tag, I have moved the todos to an issue: https://github.com/zwave-js/zwavejs2mqtt/issues/7

@chrisns