Scopes are overwritten

config = Object.assign(options, { scope: 'profile email https://www.googleapis.com/auth/plus.login' })
This overwrites any user provided scopes
Changing to config = Object.assign({ scope: 'profile email https://www.googleapis.com/auth/plus.login' }, options) would preserve expected behavior since no one can add scopes anyway, but allow the optional parameter.

did you find a solution for this?
I have the same problem. I want to add "google calendar" scope but as you wrote, it's not possible at this time.

Hey darior87!
I just switched the object assign around and it works great for me, I'm using the Google Analytics scope.
 Let me know if that works for you!

Yes, I cloned also this repo and created my own with this fix and it works. thanks to your comment!


Allow injection of custom services

Feature
Allow a user to inject their own services to the service provider.

This is implemented, but could be improved.
Update
Change implementation to match the way DSharpPlus Commands Next handles services, by passing it in from the Configuration and not directly into the client. (time insensitive.)


Add option for machinepool to create Spot MachineSets

@staebler Can you re-review it please? I have made the changes

/retest

/retest

@staebler I did make update and pushed but still the verify & e2e tests are failing, can you check the reason?

@staebler I did make update and pushed but still the verify & e2e tests are failing, can you check the reason?

Run make update (or just make update-codegen) again. You should get the following changes.
$ git diff
diff --git a/pkg/apis/hive/v1/aws/zz_generated.deepcopy.go b/pkg/apis/hive/v1/aws/zz_generated.deepcopy.go
index be122f19..e6ac0880 100644
--- a/pkg/apis/hive/v1/aws/zz_generated.deepcopy.go
+++ b/pkg/apis/hive/v1/aws/zz_generated.deepcopy.go
@@ -34,6 +34,11 @@ func (in *MachinePoolPlatform) DeepCopyInto(out *MachinePoolPlatform) {
                copy(*out, *in)
        }
        out.EC2RootVolume = in.EC2RootVolume
+       if in.SpotMarketOptions != nil {
+               in, out := &in.SpotMarketOptions, &out.SpotMarketOptions
+               *out = new(SpotMarketOptions)
+               (*in).DeepCopyInto(*out)
+       }
        return
 }

@@ -70,3 +75,24 @@ func (in *Platform) DeepCopy() *Platform {
        in.DeepCopyInto(out)
        return out
 }
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *SpotMarketOptions) DeepCopyInto(out *SpotMarketOptions) {
+       *out = *in
+       if in.MaxPrice != nil {
+               in, out := &in.MaxPrice, &out.MaxPrice
+               *out = new(string)
+               **out = **in
+       }
+       return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SpotMarketOptions.
+func (in *SpotMarketOptions) DeepCopy() *SpotMarketOptions {
+       if in == nil {
+               return nil
+       }
+       out := new(SpotMarketOptions)
+       in.DeepCopyInto(out)
+       return out
+}

@staebler I tried this but I got no changes, I have go version 1.14.6, so make update doesn't work so I ran
make update-codegen GO_REQUIRED_MIN_VERSION:=
hack/update-codegen.sh
Generating deepcopy funcs
Generating clientset for hive:v1 at github.com/openshift/hive/pkg/client/clientset
Generating listers for hive:v1 at github.com/openshift/hive/pkg/client/listers
Generating informers for hive:v1 at github.com/openshift/hive/pkg/client/informers
Generating deepcopy funcs

but there were no changes

@kahootali You need to have Hive in your GOPATH. The code generators require this. The changes are getting put under your GOPATH.

@staebler I already have Hive in my GOPATH, still cant see any change, ran
make update-codegen GO_REQUIRED_MIN_VERSION:=
make update GO_REQUIRED_MIN_VERSION:=

Repo structure
Repo Folder: /home/kahoot/go/src/github.com/kahootali/hive
echo $GOPATH
/home/kahoot/go

@kahootali Hive needs to be at /home/kahoot/go/src/github.com/openshift/hive.

/retest

/retest

@staebler Squashed & tests are also passing

@staebler done


Fix Java example at create-a-pulumi-project/index.md

The shown example won't compile, let's fix this.

thank you!!

@dixler you likely hit https://github.com/pulumi/tutorial-pulumi-fundamentals/issues/5 which I got into, the simpler solution would be to publish an image that isn't tied to arm64.


isDefaultPrevented check in editableForm prevents blur when it should occur

There's a check in the document click handler for editableForm that prevents the form from being cancelled in many cases where it should be -- for example, clicking links using ui-sref from angular ui router, or opening modals in angular ui bootstrap.
In these cases the default behavior of a link is prevented, but form blur still takes place as expected. The editable form is actually blurred and should be cancelled, but the x-editable code bails out anyway because preventDefault was called on the link.
#414 fixes the issue.

Fixed by #414


HBASE-23342 : Handle NPE while closing compressingStream

Jira: https://issues.apache.org/jira/browse/HBASE-23342

:confetti_ball: +1 overall

Vote
Subsystem
Runtime
Comment

+0 :ok:
reexec
0m 32s
Docker mode activated.

_ Prechecks _

+1 :green_heart:
dupname
0m  0s
No case conflicting files found.

+1 :green_heart:
hbaseanti
0m  0s
Patch does not have any anti-patterns.

+1 :green_heart:
@author
0m  0s
The patch does not contain any @author tags.

+1 :green_heart:
test4tests
0m  0s
The patch appears to include 1 new or modified test files.

_ master Compile Tests _

+1 :green_heart:
mvninstall
5m 33s
master passed

+1 :green_heart:
compile
0m 25s
master passed

+1 :green_heart:
checkstyle
0m 27s
master passed

+1 :green_heart:
shadedjars
4m 38s
branch has no errors when building our shaded downstream artifacts.

+1 :green_heart:
javadoc
0m 24s
master passed

+0 :ok:
spotbugs
0m 50s
Used deprecated FindBugs config; considering switching to SpotBugs.

+1 :green_heart:
findbugs
0m 47s
master passed

_ Patch Compile Tests _

+1 :green_heart:
mvninstall
4m 55s
the patch passed

+1 :green_heart:
compile
0m 22s
the patch passed

+1 :green_heart:
javac
0m 22s
the patch passed

+1 :green_heart:
checkstyle
0m 26s
the patch passed

+1 :green_heart:
whitespace
0m  0s
The patch has no whitespace issues.

+1 :green_heart:
shadedjars
4m 37s
patch has no errors when building our shaded downstream artifacts.

+1 :green_heart:
hadoopcheck
15m 40s
Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.

+1 :green_heart:
javadoc
0m 22s
the patch passed

+1 :green_heart:
findbugs
0m 55s
the patch passed

_ Other Tests _

+1 :green_heart:
unit
3m  8s
hbase-common in the patch passed.

+1 :green_heart:
asflicense
0m 15s
The patch does not generate ASF License warnings.

49m 56s

Subsystem
Report/Notes

Docker
Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-877/1/artifact/out/Dockerfile

GITHUB PR
https://github.com/apache/hbase/pull/877

Optional Tests
dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile

uname
Linux fd394c32150c 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 GNU/Linux

Build tool
maven

Personality
/home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-877/out/precommit/personality/provided.sh

git revision
master / dbbba7932c

Default Java
1.8.0_181

Test Results
https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-877/1/testReport/

Max. process+thread count
287 (vs. ulimit of 10000)

modules
C: hbase-common U: hbase-common

Console output
https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-877/1/console

versions
git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11

Powered by
Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.


ViroVRSceneNavigator show nothing on iOS

Requirements:
Please go through this checklist before opening a new issue

[X] Review the documentation
[X] Search for existing issues in: viromedia/viro & ViroCommunity/viro
[X] Use the latest ViroReact release

Environment
Please provide the following information about your environment:

Development OS: Mac
Device OS & Version: iOS 15.5
Version: ViroReact 2.23.0 and React Native 0.65.3
Device(s): iPad Pro 12.9 (2nd gen)

Description
ViroVRSceneNavigator show nothing
Reproducible Demo
const Scene1 = () => {
  return <ViroScene><ViroBox /></ViroScene>
}

<ViroVRSceneNavigator initialScene={{ scene: Scene1 }} />

Hey @hopeter1018 did you fix your problem ?

Any news? Bug still present in latest version


feat: use tokenizer.apply_chat_template() in vLLM

Description
This PR allows to use tokenizer.apply_chat_template() in the vLLM backend.
Most models contain the template definition for the OpenAI chat endpoint  in their tokenizer_config.json file on HF.
Redeclaring the template in LocalAI seems a bit counter-productive when it could be automagically applied.
This PR fixes #1989
Notes for Reviewers
I tried to do a minimal patch to ensure readability and most importantly not touch the functions code.
Doing so, I wrote a weird-ish if in core/backend/llm.go:235 that passes the OpenAI messages to the backend only if parameter is set AND the prompt is empty (so not already computed). The same kind of if can also be found in the python backend.
This ensures that everything outside of the chat endpoint (outside of functions scope) still has the predInput computed, and that it processing it will the default option, but it introduces a logic based on the prompt being empty or not that might not be completely straightforward.
Signed commits

[x] Yes, I signed my commits.

@mudler Had to fix a conflict in go.mod, I think you're gonna have to approve again.

@mudler Sorry to ping you again, but auto-merge was also disabled :(

@mudler Sorry to ping you again, but auto-merge was also disabled :(

not an issue at all! :+1:


Correcting return variable type from videoDevice::findType to be int

resolves #6256
What does this PR change?
This PR changes the return type of videoDevice::findType from unsigned int to int
This enables the correct detection of -1 as an error return code and does not allow continuation of device creation.

Hello OpenCV wonderful people,
I encountered the same issue as #6256 and fixed it. Please let me know if there's anything I've done incorrectly in the PR pipeline.
Regards,

Thanks!
Looks good to me, :+1:


Option to have this as a settings plugins

Some projects may not have a root build.gradle.kts file at all if it wasn't for this plugin. :)
Seems like this could also be applied via settings plugin, no?
This is a feature request to be able to apply this as a settings plugin.

Thanks for the issue. This is something I've long considered, but simply haven't had the time to explore.
If other people would find this useful, please 👍


[AST] Avoid possible segfault in isDirectToStorageAccess

isAccessibleFrom allows a nullptr for useDC. Thus UseDC can be a nullptr in the path checkAccess -> getAccessSemanticsFromContext -> isDirectToStorageAccess.
Resolves rdar://104620331.

@swift-ci please smoke test

@swift-ci please smoke test macOS platform

@swift-ci please smoke test macOS platform

I'm going to go ahead and merge this as-is. The change to checkAccess means that indexing should really just be checking the access level now and thus the index test wouldn't catch this anyway.


Add a Delete button for Updates

A collective admin wants to delete an update on their collective.
At the moment, there's no option for deleting it.
Include a delete update option in the edit template page.

Resolved by https://github.com/opencollective/opencollective-frontend/pull/910


The link found in Step 1 of "Team members can store their own files in OneDrive"

The link found in Step 1 of "Team members can store their own files in OneDrive" points to a location which only administrators have access to. Should it instead point to https://portal.office.com/ ?
https://docs.microsoft.com/en-us/office365/admin/setup/set-up-file-storage-and-sharing?view=o365-worldwide#team-members-can-store-their-own-files-in-onedrive

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

ID: 3c454f78-7336-f872-c945-23c2997856af
Version Independent ID: 4d7e4a09-122e-d291-769b-93a4dffb0942
Content: Set up Office 365 file storage and sharing
Content Source: Office365-Admin/setup/set-up-file-storage-and-sharing.md
Service: o365-administration
GitHub Login: @twernermsft
Microsoft Alias: twerner

@treestryder Thank you for your feedback. We have made the change in document via PR #879
Thanks
Sri

@treestryder - From our understanding, the issue has been resolved based on this merged commit https://github.com/MicrosoftDocs/OfficeDocs-O365ITPro/commit/66f58b74c4990bc9ed05a61734a30730f174a8c6.
Thank you for your contribution to make the docs better! Much appreciated!


Investigate consolidation of jobs on fewer VMs

We have historically been divergent from cf-deployment (because we came first, and they didn't take any direction from us when they wrote their configuration).
It's time to take another look at cf-deployment and see if they have any neat tricks up their sleeves for corralling CF on fewer VMs (even in the non-small-footprint model).
Attached are two summary YAMLs that extract information on instance groups and their constituent jobs, for both a cf-deployment (with no additional ops files activated) and a Genesis CF.
cf-deployment-instance-groups.yml.txt
genesis-cf-instance-groups.yml.txt

(for posterity, here's the command I ran to generate these, assuming manifest.yml is either the root cf-deployment.yml file from cf-deployment, or a full BOSH manifest from a Genesis-deployed CF:)
spruce json manifest.yml  | \
  jq -r '{instance_groups: [.instance_groups[] | {name: .name, zjobs: [.jobs[] | (.release + "/" + .name)] }]}' | \
  spruce merge | \
  sed -e 's/^  zj/  j/'

as we're moving to matching cf-deployment in its entirety, this ticket is moot.


Add pull request template

Is your feature request related to a problem? Please describe.
This repo does not contain a Pull Request template.
Describe the solution you'd like
I would like to add a Pull request template so that if someone wants to raise a pr they can select the right template and contribute.
Describe alternatives you've considered
NA
Additional context
I want to contribute to this repo for Hactoberfest21. Kindly assign this issue to me!

@gauravsinhaweb check out this issue, please.

@gauravsinhaweb can I work on this issue?

@SrishtiArya07 Yes, go for it 👍

@gauravsinhaweb Please check out the pr #42  I raised for this issue. Tell me if you need any changes.


Fixwarnings

Fix compiler & linker warnings.
Add missing dependency that prevented NativeCore from being automatically built for some projects.

Just that one issue I found, otherwise it's good. Fix the one thing and I'll merge it.

That was fast :^)   %p always prints hex values, so I added the 0x to make that obvious.  I can change the print format back to %d (so we get decimal output) and then cast the arg. to (int), which satisfies the compiler.  But will the cast create an issue in the future if someone tries to compile SageLauncher for 64-bit?

cast by "uintptr_t". the compiler takes care of 64-bit / 32-bits
On Thu, Oct 15, 2015 at 11:09 AM, Keith Fischer<EMAIL_ADDRESS>wrote:

That was fast :^) %p always prints hex values, so I added the 0x to make
that obvious. I can change the print format back to %d (so we get decimal
output) and then cast the arg. to (int), which satisfies the compiler. But
will the cast create an issue in the future if someone tries to compile
SageLauncher for 64-bit?
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148476232.

And the Java code is parsing it as a 64 bit value as well. :)

Casting to (uintptr_t) makes the compiler happy.  Will resubmit shortly.

Should be good now.

could be this way to make the compiler happy on both 32-bit/64-bit?
sprintf(winBuf, "%lld", (uint64_t)(uintptr_t)hWnd);
On Thu, Oct 15, 2015 at 11:45 AM, Keith Fischer<EMAIL_ADDRESS>wrote:

Should be good now.
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148485298.

Yeah, this should be printed as a 64-bit value for compatibility.

If the goal is to get a decimal value, then it seems like we don't need the double cast.
sprintf(winBuf, "%lld", (uint64_t) hWnd);  should produce what we want.

Yeah, it's better.
On Thu, Oct 15, 2015 at 11:55 AM, Keith Fischer<EMAIL_ADDRESS>wrote:

If the goal is to get a decimal value, then it seems like we don't need
the double cast.
sprintf(winBuf, "%lld", (uint64_t) hWnd); should produce what we want.
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148487731.

wait, I make a quick test with VC 2015 that gets different results in a
special case of the x86 and hWnd < 0 (such 0x890abcde)
sprintf_s(tmp, 64, "%lld", (uint64_t)((uintptr_t)hWnd));
sprintf_s(tmp, 64, "%lld", (uint64_t)(hWnd));
So, it's better to have (uint64_t)((uintptr_t)hWnd)
On Thu, Oct 15, 2015 at 11:57 AM, Qian Zhang<EMAIL_ADDRESS>wrote:

Yeah, it's better.
On Thu, Oct 15, 2015 at 11:55 AM, Keith Fischer<EMAIL_ADDRESS>wrote:

If the goal is to get a decimal value, then it seems like we don't need
the double cast.
sprintf(winBuf, "%lld", (uint64_t) hWnd); should produce what we want.
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148487731.

Wait...why are unsigned integers being used here? These need to translate
to 64 bit signed integers since that's what Java will be reading them as
and then passing them back to native code that way as well (and then those
are directly cast to pointers when used).
On Thu, Oct 15, 2015 at 12:09 PM, qianzhang5<EMAIL_ADDRESS>wrote:

wait, I make a quick test with VC 2015 that gets different results in a
special case of the x86 and hWnd < 0 (such 0x890abcde)
sprintf_s(tmp, 64, "%lld", (uint64_t)((uintptr_t)hWnd));
sprintf_s(tmp, 64, "%lld", (uint64_t)(hWnd));
So, it's better to have (uint64_t)((uintptr_t)hWnd)
On Thu, Oct 15, 2015 at 11:57 AM, Qian Zhang<EMAIL_ADDRESS>wrote:

Yeah, it's better.
On Thu, Oct 15, 2015 at 11:55 AM, Keith Fischer <
<EMAIL_ADDRESS>wrote:

If the goal is to get a decimal value, then it seems like we don't need
the double cast.
sprintf(winBuf, "%lld", (uint64_t) hWnd); should produce what we want.
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148487731.

—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148491232.

--
Jeffrey Kardatzke
<EMAIL_ADDRESS>Google, Inc.

It's fine passing hWND back forth code that are 32-bit or 64-bit. If a
32-bit hWND is passed into a 64-bit code(plugin), there would be a problem,
this situation could not exist?.
On Thu, Oct 15, 2015 at 1:16 PM, Jeffrey Kardatzke<EMAIL_ADDRESS>
wrote:

Wait...why are unsigned integers being used here? These need to translate
to 64 bit signed integers since that's what Java will be reading them as
and then passing them back to native code that way as well (and then those
are directly cast to pointers when used).
On Thu, Oct 15, 2015 at 12:09 PM, qianzhang5<EMAIL_ADDRESS>wrote:

wait, I make a quick test with VC 2015 that gets different results in a
special case of the x86 and hWnd < 0 (such 0x890abcde)
sprintf_s(tmp, 64, "%lld", (uint64_t)((uintptr_t)hWnd));
sprintf_s(tmp, 64, "%lld", (uint64_t)(hWnd));
So, it's better to have (uint64_t)((uintptr_t)hWnd)
On Thu, Oct 15, 2015 at 11:57 AM, Qian Zhang<EMAIL_ADDRESS>wrote:

Yeah, it's better.
On Thu, Oct 15, 2015 at 11:55 AM, Keith Fischer <
<EMAIL_ADDRESS>wrote:

If the goal is to get a decimal value, then it seems like we don't
need
the double cast.
sprintf(winBuf, "%lld", (uint64_t) hWnd); should produce what we want.
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148487731.

—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148491232.

--
Jeffrey Kardatzke
<EMAIL_ADDRESS>Google, Inc.
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148508317.

Yeah, seems like "unsigned" will cause big problems, for both 32 and 64 bit environments..   Admittedly, I'm not familiar with the Java code.  What's the type of the consumer on the Java side... is it int64, long long, or something else?   Shouldn't the cast on the native side  match the type on the Java side?
And I still don't see the point to the double cast.  What am I missing?

There is no downside with double cast (unsigned) for java, so I say it's
better...
On Thu, Oct 15, 2015 at 1:36 PM, Keith Fischer<EMAIL_ADDRESS>wrote:

Yeah, seems like "unsigned" will cause big problems, for both 32 and 64
bit environments.. Admittedly, I'm not familiar with the Java code. What's
the type of the consumer on the Java side... is it int64, long long, or
something else? Shouldn't the cast on the native side match the type on the
Java side?
And I still don't see the point to the double cast. What am I missing?
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148513008.

Qian,
There won't ever be a mix of 32 bit and 64 bit. Whatever architecture the EXE is compiled for; the DLLs must use the same thing...it will all be 32 bit or 64 bit; never a mixture of both.
We CANNOT use uint64_t (or any other unsigned types here) because then it can overflow the value when Java stores it.
It should be just cast directly to a int64_t.  The reason Qian saw a difference in the double cast vs. single cast is that the uintptr_t cast is guaranteed to not lose any precision...while the uint64_t cast can lose precision due to values being negative.

I agree: Qian's test results were colored by the signed/unsigned issue.
Jeff:  the remaining question is: would it be better to cast to (long long), or should it instead be cast to (int64_t) ??    Nitpicking, perhaps, but I'd like to do this properly.
I tried looking to see where the value gets cast back to a pointer and de-referenced, but I'm not sure how to go about finding it.  Can you point me in the right direction?

You can see an example in here:
https://github.com/google/sagetv/blob/master/native/dll/Win32DLL/SageTVWin32DLL.cpp
In that first function, it's casting a jlong (64 bit signed integer in
Java) to an HWND.  That jlong is created by parsing the string that comes
from what we've been discussing here. :)
I'd prefer you use int64_t instead of long long just because that's been
beaten into my head at Google as the right way to do it. :) Ideally, the
sprintf statement would also use PRId64 for the type specifier rather than
lld...but I don't think that convention is being used elsewhere in the code
currently so I don't think we need to go down that path yet.
On Thu, Oct 15, 2015 at 2:41 PM, Keith Fischer<EMAIL_ADDRESS>wrote:

I agree: Qian's test results were colored by the signed/unsigned issue.
Jeff: the remaining question is: would it be better to cast to (long
long), or should it instead be cast to (int64_t) ?? Nitpicking, perhaps,
but I'd like to do this properly.
I tried looking to see where the value gets cast back to a pointer and
de-referenced, but I'm not sure how to go about finding it. Can you point
me in the right direction?
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148528817.

--
Jeffrey Kardatzke
<EMAIL_ADDRESS>Google, Inc.

I see in Java_sage_Sage_postMessage0() in SageTVWin32DLL.cpp that the relevant value WinID is typed as jlong (which itself is #typedef'd long long).   Should we use jlong in the cast we're discussing?
BTW, I long ago learned the virtue of using int64_t (etc) instead long long.  But in this particular case it's not quite as clear-cut about which is best, since the value tunnels in & out of the Java domain.  Does the Java code actually operate on that value, or is it pretty much a pass-thru?

Great point...yeah...cast it to a jlong. :)  And the Java code is just
passing this value through.
On Thu, Oct 15, 2015 at 3:13 PM, Keith Fischer<EMAIL_ADDRESS>wrote:

I see in Java_sage_Sage_postMessage0() in SageTVWin32DLL.cpp that the
relevant value WinID is typed as jlong (which itself is #typedef'd long
long). Should we use jlong in the cast we're discussing?
BTW, I long ago learned the virtue of using int64_t (etc) instead long
long. But in this particular case it's not quite as clear-cut about which
is best, since the value tunnels in & out of the Java domain. Does the Java
code actually operate on that value, or is it pretty much a pass-thru?
—
Reply to this email directly or view it on GitHub
https://github.com/google/sagetv/pull/47#issuecomment-148538193.

--
Jeffrey Kardatzke
<EMAIL_ADDRESS>Google, Inc.


log/slog: JSONHandler should deduplicate keys

What version of Go are you using (go version)?
8edcdddb23c6d3f786b465c43b49e8d9a0015082
Does this issue reproduce with the latest release?
yes
What did you do?
	logger := slog.New(slog.NewJSONHandler(os.Stdout))
	logger.LogAttrs(
		context.Background(),
		slog.LevelWarn,
		"fancy message",
		slog.Bool("msg", false),
		slog.String("my attr", "something"),
	)

What did you expect to see?
JSONHandler should be fixed by deduplicating those special keys (time, message, level, and source).
I use "source" as a key in a lot of my code right now, so I'd run into this problem.
Either
{
	"time": "2023-04-01T04:27:46.0959443-06:00",
	"level": "WARN",
	"msg": "fancy message",
	"my attr": "something"
}

Or
{
	"time": "2023-04-01T04:27:46.0959443-06:00",
	"level": "WARN",
	"msg": "fancy message",
	"msg.msg": false,
	"my attr": "something"
}

Or
{
	"time": "2023-04-01T04:27:46.0959443-06:00",
	"level": "WARN",
	"msg": "fancy message",
	"fields": {
		"msg": false,
		"my attr": "something"
	}
}

What did you see instead?
Invalid json.  It would break our log aggregation.  Default/Builtin handlers should never create invalid output.
{
	"time": "2023-04-01T04:27:46.0959443-06:00",
	"level": "WARN",
	"msg": "fancy message",
	"msg": false,
	"my attr": "something"
}

@jba

Whatever solution is chosen here, I would argue that it should work even if the user explicitly provides duplicate names:
logger.LogAttrs(
	context.Background(),
	slog.LevelWarn,
	"fancy message",
	slog.String("duplicate", "something"),
	slog.String("duplicate", "something"),
	slog.String("duplicate", "something"),
	slog.String("duplicate", "something"),
	slog.String("duplicate", "something"),
)

One possible solution is to use a "#%d" scheme similar to how testing.T.Run deduplicates conflicting test names.

I think the current situation is fine, duplicate keys aren't invalid, just less interoperable.
Changing the key name to deduplicate it seems worse.
As for the original issue, I'll note that option 3 was why WithGroup was added (though this doesn't affect deduplication in general).

duplicate keys aren't invalid, just less interoperable.

Technically you are correct. Under RFC 8259, section 4, duplicate names are essentially treated as undefined behavior.
In a future major version of "encoding/json", we will likely make duplicate keys an explicit error (which is still complaint behavior with RFC 8259) as this lack of checking has been exploited, resulting in severe security vulnerabilities (e.g., CVE-2017-12635). It's unlikely that duplicate names will have such drastic security implications in logging, but strict checking of duplicate names is likely the overall direction that JSON will gradually move towards.
As an aside, Tim Bray, the author of RFC 8259 actually recommends against naïve compliance with RFC 8259, but rather that implementations adhere to RFC 7493 which makes strict decisions on issues that RFC 8259 leaves as undefined.

I think it's important to understand whether encoding/json v2 with its currently proposed semantics is likely to be the accepted version. We should probably align slog with those semantics, even if slog appears in Go before encoding/json v2. We're already doing that for formatting NaNs, infinities and, probably, durations (#59345).
For this issue, if we think no dups will be the default for encoding/json v2, then I wouldn't be opposed to making it the default in slog and adding an AllowDuplicateNames option. And we should do the same for TextHandler for consistency.

whether encoding/json v2 with its currently proposed semantics is likely to be the accepted version

That I can't really answer unfortunately.
We haven't even started the GitHub discussion on anything related to v2 "encoding/json" and may not be ready to do so for some months.  That's perhaps the hardest part and requires quite a lot of head-scratching.
That said, if there is a v2 "encoding/json", I feel like the security arguments for rejecting duplicate names by default is pretty compelling.

The (very) tentative plan for v2 is that v1 "encoding/json" will actually be implemented in terms of v2 with the appropriate number of options specified that makes it behave semantically identically to what v1 "json" does today. Essentially, with the appropriate set of options, you can configure the semantics to be any hybrid mix that is in-between the default behavior of v1 and the default behavior of v2.
Since "log/slog" already depends the v1 semantics of "encoding/json", you could imagine doing nothing to address this issue for now. I don't think we should block adoption of "slog" on a hypothetical v2 "json".
Supposing v2 "json" one day lands, you could imagine that v2 "json" could be integrated into "slog" by exposing something like:
package slog

type HandlerOptions struct {
    // JSONOptions is a list of Option
    // that configures how JSON is marshaled.
    JSONOptions []json.Option
}

In order to provide v2 "json" support for "slog", you could imagine "slog" doing something like:
// Start with DefaultV1Options since "slog" has historically
// marshaled with mostly v1 semantics by default.
opts := []json.Option{json.DefaultV1Options}
// We have historically specified Encoder.SetEscapeHTML(false).
// By passing a nil escape function, we achieve the same behavior.
opts = append(opts, json.EscapeRune(nil))
// Append any user-specified options,
// which will override any previously set options.
opts = append(opts, h.opts.JSONOptions...)
return json.Marshal(v, opts...)

(You'll note that the options API for "json" above is different than what currently exists in "go-json-experiment". I mentioned elsewhere in #59339 that we're considering switching to variadic options as we see it as one of the ways to resolve the v1-to-v2 compatibility tensions).
Thus, for someone to use v2 "json" with "slog", they would do something like:
slog.HandlerOptions{
    JSONOptions: []json.Option{json.DefaultV2Options},
}.NewJSONHandler(...)

I should also note that DefaultV1Options and DefaultV2Options represent a composite set of options. For example DefaultV1Options includes AllowDuplicateNames(true) while DefaultV2Options includes AllowDuplicateNames(false).
One could imagine that "slog" inspects HandlerOptions.JSONOptions and checks whether AllowDuplicateNames is true or false:

If false, it does what it currently does (i.e., just emits duplicate object members)
If true, it does automatic name-deduplication similar to what I proposed in https://github.com/golang/go/issues/59365#issuecomment-1494730820.

Thanks, @dsnet, that makes a lot of sense.

It would be very unexpected if repeated calls to Log with the same key "duplicate" were to produce output like {"duplicate": "alpha", "duplicate#01": "bravo", "duplicate#02": "charlie"}. Among many other reasons, this behavior would make it impossible to reliably assert that a log event emitted with some key K would actually be rendered with that key K in the ultimate output format.

It would be very unexpected if repeated calls to Log with the same key "duplicate" were to produce output like {"duplicate": "alpha", "duplicate#01": "bravo", "duplicate#02": "charlie"}.

Possibly, but there are no good answers when it comes to duplicate names.
It would be nice if the API made it statically impossible to have duplicates,
but I don't see how that can be done without taking in a map[string]any as the input,
which would not be performant.
Given the possibility of runtime duplicates, your options are:

Emit the object with all duplicate fields
Emit the object, but only keep one of the duplicate fields (e.g., the first occurrence)
Rename the duplicate fields to avoid a conflict
Report an error
a. and still do either 1, 2, or 3
b. or simply reject the message and not log it.

With option 1, you risk log parser not being able to read the message entirely or accurately. Duplicate names result in undefined behavior where the parser might ignore duplicates, only use the first one, only use the last one, or use some merged form of all duplicates.
With option 2, you avoid any ambiguities when reading, but are subtly dropping information when writing.
With option 3, you avoid any ambiguities as to the meaning when reading, but are changing the meaning when writing. At least the meaning when reading is consistent across all parsers.
Option 4a is decent as it allows the caller to at least realize that they're possibly holding the API wrong, but I suspect most people will not check the error of logging calls.
Of all the bad options, option 4a with either 1 or 3 is probably the best approach.
Options 2 and 4b discard information, which seems antithetical to the point of a logging library.

Option 2 is pretty clearly the correct choice.

Options 2 and 4b discard information, which seems antithetical to the point of a logging library.

I'm not sure why anyone would expect a structured logging library to never discard information. Emitting multiple structured log events with the same key is pretty clearly going to resolve to a single "winner" for that key, most likely the last event, but really any event would be acceptably valid.

Option 2 is pretty clearly the correct choice.

That's a matter of opinion. Just as how you find option 3 an odd position, I find option 2 an odd position. Every option has pros and cons.

I'm not sure why anyone would expect a structured logging library to never discard information.

Any value passed to LogAttrs by the programmer carries fairly clear intent that it be logged. Arbitrarily choosing some field to keep, while discarding all others goes against the intent.

Values passed to a structured logging library necessarily include a key which is, by definition, understood as unique within the target log event. So, no: it's definitely not the case that calling a structured logging method like LogAttrs can be assumed to produce any specific log output. Structured logging is, hopefully obviously, not equivalent to log.Printf, or fmt.Printf, or anything like that.

@dsnet For option 4, how would an error be reported?  A separate log line at warning level, or some out-of-band channel or something?
Personally I would have probably expected option 2, where the latest value wins (except that the Message, Time, Level, and Source fields would always win over any Attribute values).
My preferences, in order, would probably be:
option 2 or 4a2
option 3 or 4a3
option 1 would break a lot of log aggregation systems
option 4b would seem like the worse option, as you lose the log line completely, making any error even harder to diagnose

Values passed to a structured logging library necessarily include a key which is, by definition, understood as unique within the target log event

By technical definition, I agree. However, in practice, duplicates often occur due to human error. If I make a mistake, I would rather still have all duplicates logged (whether with option 1 or 3).
As an aside, we should definitely create a vet check that detects accidental duplicate names.

@dsnet For option 4, how would an error be reported? A separate log line at warning level, or some out-of-band channel or something?

I was thinking of an explicit error returned by slog.Handler.Handle. I guess a separate log line is a variation of option 4. However, a go vet check might be a better way to statically detect most of these cases.

Values passed to a structured logging library necessarily include a key which is, by definition, understood as unique within the target log event

By technical definition, I agree. However, in practice, duplicates often occur due to human error. If I make a mistake, I would rather still have all duplicates logged (whether with option 1 or 3).

I'm not sure how to evaluate whether or not a particular mistake made by consumers of an API is common enough to warrant changing the API to accommodate that mistake. Especially so if that change would violate the intuitive expectations of the API, as is the case here.

It would be nice if the API made it statically impossible to have duplicates, but I don't see how that can be done without taking in a map[string]any as the input, which would not be performant.

I agree, I think a map is at least very close to the ideal data structure here. Regarding performance, the current implementation of x/exp/slog/internal/buffer allocates output buffers as a sync.Pool resource.
type Buffer []byte

It's possible to also pool a map[string]bool for keys, and a []string for groups
type Buffer struct{
	buf []byte
	keys map[string]bool
	group []string
}

On freeing the Buffer, like the buf slice, the keys and group fields can be emptied without releasing backing memory. I used something similar experimenting with string interpolation of attributes into log messages which was fun and way more useless in practice than I anticipated, but I did benchmark and this Buffer approach seemed to:

not allocate
increase the time of logging proportional to the number of attributes (loosely and non-scientifically, each Attr was 20% more expensive - use 5 Attrs and pay for 6)
increase the size of pooled objects by a constant factor - the additional fields are small, but so is the buf slice.

A contrasting approach: I think a Deduplicate wrapper for ReplaceAttr would be possible without any change to slog, although tricky because deduplication after ReplaceAttr seems significant. Notably, with no change to slog, I don't think this allows for your option 4 of reporting an error, which I thought was intriguing. Something like:
func Deduplicate(func([]string, Attr)(Attr)) func([]string, Attr) Attr
(I really don't have any opinion on what's necessary here - I don't see much need for deduplication personally, prior art in structured logging in Go doesn't deduplicate AFAIK, and yet the the arguments for deduplication seem pretty reasonable)

Values passed to a structured logging library necessarily include a key which is, by definition, understood as unique within the target log event

By technical definition, I agree. However, in practice, duplicates often occur due to human error. If I make a mistake, I would rather still have all duplicates logged (whether with option 1 or 3).
As an aside, we should definitely create a vet check that detects accidental duplicate names.

Would that be able to detect duplicates in sub-packages though?  I feel like the most common occurrence of duplicates will be adding attributes and telemetry before passing the logger down to libraries or other sub-packages.

@dsnet For option 4, how would an error be reported? A separate log line at warning level, or some out-of-band channel or something?

I was thinking of an explicit error returned by slog.Handler.Handle. I guess a separate log line is a variation of option 4. However, a go vet check might be a better way to statically detect most of these cases.

Right now, errors coming from Handler are discarded by the most common slog methods (ie: Info, WarnCtx, etc).  I certainly wouldn't want to change the signature of Info, WarnCtx, etc, to include returning the error.  I suppose it is possible to use a middleware type handler that captures and does something with errors, but that wouldn't be something people would use by default.
In any case, I would still want the log line outputted before any error returned.

Would that be able to detect duplicates in sub-packages though?

Depends on the complexity of call-tracking. This is a similar problem as vet checks for fmt.Sprintf, which already has some degree of call tracking. I don't know if it descends through package boundaries, but it is able to cross function call boundaries.

One option might be to rename duplicated keys to !BADKEY(duplicateKey) as a sort of combination of @dsnet's options 3 and 4a while carrying some symmetry with the existing error-while-logging mechanism. Then again, there is also some asymmetry if it is only JSONHandler that checks duplicates.

Another argument against option 2 (which discards data) is security.
If untrusted input is able affect what names are present, then that can be used as a mechanism to hide malicious activity, knowing that it will not be logged. IMO, it seems better for readers to possibly choke on the malformed messages, than for the malicious activity to not be properly logged. An human investigator looking at the malformed message can see the duplicate shenanigan at play.

@08d2

There are many additional (and unavoidable) security concerns in a structured logging API beyond this one

Like what? The other security issue I can imagine is a DOS attack where you're tricked into logging something massive, but that's a distinctly different class of security issue from being able to hide activity or perform remote-code-execution.
@AndrewHarrisSPU

I think a map is at least very close to the ideal data structure here. Regarding performance, the current implementation of x/exp/slog/internal/buffer allocates output buffers as a sync.Pool resource.

I'm not sure I understand. Suppose we used a Go map such that you did something like:
slog.Info(map[string]any{
    "msg":   "my wonderful message",
    "other": "something else",
})

Yes, you get the benefit of static compilation preventing duplicate map keys (although it can't check the implicit "time" and "level" keys). However, I'm missing the part where the pool helps out.
When slog.Info calls the underlying slog.Handler.Handle, it goes through an interface method call, and all escape analysis goes out the window, so the input map is treated as having escaped (and thus heap-allocated). To ensure the input remains stack allocated, you would have to copy the input map to one of the pooled maps and then call Handler.Handle with it. It's not clear to me that the act of copying the map is going to be fast enough.

@dsnet

When slog.Info calls the underlying slog.Handler.Handle, it goes through an interface method call, and all escape analysis goes out the window, so the input map is treated as having escaped (and thus heap-allocated). To ensure the input remains stack allocated, you would have to copy the input map to one of the pooled maps and then call Handler.Handle with it. It's not clear to me that the act of copying the map is going to be fast enough.

I don't think we need to input maps to achieve option (2) or (3), I think I should clarify. This technique doesn't involve any change that isn't internal to Handler implementation, everything else looks the same as before. There's no map copying; there is one blank, probably already-allocated map obtained from a pool when the output Buffer is obtained by Handler.Handle.
This map can be populated with keys visited when the handler traverses the attributes of the record, writing to output. A couple of quirks are that the traversal is made more interesting and dynamic by things like group Attrs, LogValuers that expand into groups, or ReplaceAttrs, etc. ReplaceAttr needs to happen before a key goes into the map, but this is tractable. Also, precisely, it's not just key strings that need to be matched, it's also the group prefix - a little tricky but possible to do safely and without allocating.
If this map is populated with visited keys, we can detect whether some key has been committed previously in the log line, and trigger behavior option (2) or (3) as a result.

Like what? The other security issue I can imagine is a DOS attack where you're tricked into logging something massive, but that's a distinctly different class of security issue from being able to hide activity or perform remote-code-execution.

Consider a type which renders in one way in some version X, and a different (and richer) way in a subsequent version Y. Consider the case where that change results in sensitive information from that type which was previously not logged becoming logged. This happens easily if the type grows a new sensitive field in a new version. Such changes are in practice never considered breaking in terms of versioning. A downstream application which logs a value of that type will have one behavior if it includes version X of the relevant package, and a different behavior when it includes version Y, and that version upgrade is in practice never going to be noticed, or tested, by the downstream application.
Printf based logging does not generally have this problem, because it is not conventional to pass values directly as a Printf log arguments.

fwiw we once used a database driver that logged our database password in plain text when in debug mode, using regular old Printf (it was patched right away)
I don't really see any security issues with structured logging that don't already exist with regular logging.
And its not like this is log4j and we are executing the statements in the logs or trying resolve remote xml or something.  If the data is strings, the worst case I can think of is accidental logging of sensitive data.

A few points:
@veqryn's specific problem, duplicate "source" keys, will be easy to resolve with a ReplaceAttr function once #59280 is approved. You can then distinguish a "source" attribute that comes from the handler by seeing if the value is a *slog.Source. It's also going to appear before a user-supplied attribute, a fact you can use right now. The design doesn't promise that that will always be true, but I can't see why we'd change it.
In general, if you don't like duplicate keys, you can write a wrapping Handler. That can return an error if you like, but it's worth remembering that logging is often the last resort for debugging in the wild. If you've ever been in a position where you can't debug because you have a picky logger that refuses to output everything it has, you know how frustrating that can be, and how happily you'd jettison your principles of careful error detection and failing fast for just a few more characters on the screen.
This isn't a proposal, but if it were I'd vote to decline. We can always add dedup functionality later (though not as the default, it's true). As for json v2 compatibility, @dsnet has provided a good blueprint for how to achieve that when the time comes. But you go into the standard library with the packages you have, not the packages you wish you had. slog should behave like the current encoding/json.

What if the fields are changed to dupe.1 dupe.2 etc but to keep it from going unnoticed there’s a new field added with a canonical name like “duplicated-fields”: [“dupe”] and if you see anything called duplicated fields in your logs you know to go figure out what’s misconfigured.

I arrived here after seeing weird stuff on the logs. I thought that doing something like this I would be able to override the previously set attribute:
package main

import (
	"log/slog"
	"os"
)

func main() {
	first := slog.New(slog.NewJSONHandler(os.Stdout, nil)).With("tag", "fooo")
	first.Info("hello")

	second := first.With("tag", "bar")
	second.Info("bye")
}

// {"time":"2009-11-10T23:00:00Z","level":"INFO","msg":"hello","tag":"fooo"}
// {"time":"2009-11-10T23:00:00Z","level":"INFO","msg":"bye","tag":"fooo","tag":"bar"}

In our logs, it looks like the fields are being "merged", with parts of words from different attributes. e.g. start-activityware  from start-activity and logger-middleware.
Is attribute overriding something supported? I wanted to use it to change the "component" name in the logs. Something like "foo-handler", "foo-service" etc.

Scrambled text could result from racing writes to os.Stdout. IIRC this shouldn’t happen with the code you’ve listed(?) but just to toss out an idea quickly.

@AndrewHarrisSPU indeed, I was creating multiple handlers in a middleware, so each request had its own mutex to os.Stdout. Thank you! It makes a lot of sense.

Thanks @AndrewHarrisSPU for that quick and accurate diagnosis. @dbofmmbt, there's nothing we can do about that multiple-mutex issue, unfortunately. In fact the original log package has the same issue. I think the best solution is to wrap your io.Writer in one that acquires the mutex during Write, then install the wrapped io.Writer in multiple loggers. A well-behaved handler should call Write once per log event, and the built-in ones do, though that is not a hard requirement.
About deduping or overriding, I don't think the default handlers will add it. It's expensive, especially combined with pre-formatting. You can easily write your own wrapping Handler whose WithAttrs method keeps a map, so calls to Logger.With will override previous values. You'll give up pre-formatting (since you can't format anything in that map until Handle is called) but you may not need the extra speed.

The lack of deduplication makes this completely useless and non-valid JSON/JS. Many developers set logging keys in middleware to attach information for each request. I am seeing 1000 duplicate KEYS which are considered unique in every other logging library except this one? Is this a april fools joke?

@codeliger, I'll let others speak to the utility of the built-in JSON handler. But duplicate keys are valid JSON, according to the RFC.

@jba would you mind informing the section that the RFC specifies duplicated fields? I only found this:

When the names within an object are not unique, the behavior of software that receives such an object is unpredictable. Many implementations report the last name/value pair only. Other implementations report an error or fail to parse the object, and some implementations report all of the name/value pairs, including duplicates.

This piece is not encouraging in regards to generating duplicated fields in the stdlib's JSONHandler, IMO.

@dbofmmbt "unpredictable" is still standardised. The same way like - what if you have a 50 digit long integer. It's standard compliant, yet not every parser would be able to parse it or even when they do - they might interpret it differently.

@dbofmmbt

This piece is not encouraging in regards to generating duplicated fields in the stdlib's JSONHandler, IMO, since its behavior looks unspecified.

That's exactly it, it's unspecified by JSON:
The RFC qualifies that:
The names within an object SHOULD be unique. ...

with SHOULD more precisely defined:
3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
   may exist valid reasons in particular circumstances to ignore a
   particular item, but the full implications must be understood and
   carefully weighed before choosing a different course.

Relative to other Go logging libraries: duplicate keys are possible with slog, zap and zerolog, and last-seen-wins in logrus. This could be seen as driven by performance concerns, in contrast to the former's (fast) data models, logrus uses (expensive) maps. But it's not the case that logrus is doing something that is more correct according to the JSON specification, the RFC positively identifies it as only one possible implementation, with no correct implementation identified.

Hey all, I've created a logging handler middleware that will deduplicate attribute keys, before they go to the json logger.
Please check it out: https://github.com/veqryn/slog-dedup
go get github.com/veqryn/slog-dedup
Overwrite Older Duplicates Handler
logger := slog.New(dedup.NewOverwriteHandler(slog.NewJSONHandler(os.Stdout, nil), nil))
logger.Info("this is the dedup overwrite handler",
    slog.String("duplicated", "zero"),
    slog.String("duplicated", "one"),
    slog.String("duplicated", "two"),
)

Outputs:
{
    "time": "2023-10-03T01:30:00Z",
    "level": "INFO",
    "msg": "this is the dedup overwrite handler",
    "duplicated": "two"
}

Ignore Newer Duplicates Handler
logger := slog.New(dedup.NewIgnoreHandler(slog.NewJSONHandler(os.Stdout, nil), nil))
logger.Info("this is the dedup ignore handler",
    slog.String("duplicated", "zero"),
    slog.String("duplicated", "one"),
    slog.String("duplicated", "two"),
)

Outputs:
{
    "time": "2023-10-03T01:30:00Z",
    "level": "INFO",
    "msg": "this is the dedup ignore handler",
    "duplicated": "zero"
}

Increment Newer Duplicate Key Names Handler
logger := slog.New(dedup.NewIncrementHandler(slog.NewJSONHandler(os.Stdout, nil), nil))
logger.Info("this is the dedup incrementer handler",
    slog.String("duplicated", "zero"),
    slog.String("duplicated", "one"),
    slog.String("duplicated", "two"),
)

Outputs:
{
    "time": "2023-10-03T01:30:00Z",
    "level": "INFO",
    "msg": "this is the dedup incrementer handler",
    "duplicated": "zero",
    "duplicated#01": "one",
    "duplicated#02": "two"
}

Append Duplicates Together in an Array Handler
logger := slog.New(dedup.NewAppendHandler(slog.NewJSONHandler(os.Stdout, nil), nil))
logger.Info("this is the dedup appender handler",
    slog.String("duplicated", "zero"),
    slog.String("duplicated", "one"),
    slog.String("duplicated", "two"),
)

Outputs:
{
    "time": "2023-10-03T01:30:00Z",
    "level": "INFO",
    "msg": "this is the dedup appender handler",
    "duplicated": [
      "zero",
      "one",
      "two"
    ]
}

@jba can I get this added to the slog wiki please? https://github.com/golang/go/wiki/Resources-for-slog

IMO it should be one of the JSONHandler's options. JSON format says it SHOULDN'T contain duplicate keys, but it's not explicitly forbidden.
But, imagine your log processor is written in Go, using the encoding/json/v2 implementation, which is not allowing duplicate keys, and you will lose some logs due to the duplicates. Very probably you will find out in the worst possible moment, looking for the important logs.
Because the ship has sailed and log/slog is in Go 1.21, and also because it has performance implications, it should be opt-in. Normally, I would be for opt-out, because it would mimic the behavior of github.com/sirupsen/logrus, but it's not an option now.

encoding/json/v2 will have an option for allowing duplicates. See the discussion.
When/if encoding/json/v2 is in the standard library, we will find a way for slog to use it. Maybe we'll have a JSONHandlerV2, or maybe we'll add a list of options to the Options struct.

@jba Sorry, I wasn't clear.
What I meant is any log processor in any language what doesn't support duplicate keys.
Let's change our example to NodeJS, what seems to not support duplicate keys as well.
https://stackoverflow.com/questions/30842675/how-to-get-the-json-with-duplicate-keys-completely-in-javascript
Most probably it will be written in C/C++/Rust/Go, but you know, it's just an example. And I'm also not very aware of the JSON lib situation in those ecosystems.

I'm migrating our codebase from logrus to slog, and I'm really not sure if our codebase is "correct", so I'm adding the dedup.NewOverwriteHandler to achieve the same logrus-like behavior. At least from the start.

@prochac Using one of @veqryn's dedup handlers is a good way to address your concerns.

Yes, that's definitely what I'm doing at this moment, with tuning HandlerOptions.ReplaceAttr to catch some other quirks.
I just thought it would be worth having in the std lib. It's a good safety net, if people understand the performance implications.

Duplicate keys can lost if the processor doesn't accept duplicates, but they can also be lost if the processor expects mykey and gets mykey-1. There's not a best option here.

@carlmjohnson Not keys, the whole log line can be dropped as an invalid JSON, if the JSON parser considers duplicate keys as an invalid JSON object

@proachac. There are no good answers when it comes to duplicate names.
You mentioned that you view your logs through Grafana. What is the underlying data store that you're using with Grafana, and does that at least store JSON objects with duplicate names?
In particular, I'm trying to understand whether this a storage problem or a reading problem:

A storage problem is one where Prometheus (a common data store for Grafana) refuses to store the log message and drops it entirely. In this situation, there is no hope for recovery since the data is lost forever.
A reading problem is one where some application writing in a programming language of choice can't read the data. The default behavior of v2 "json" will reject values with duplicate names, but this can be easily disabled with json.Unmarshal(b, &v, jsontext.AllowDuplicateNames(true)). That said, the semantic behavior of duplicate names will differ across different implementations. In this situation, the situation is bad, but at least there is hope for recovering the logs data.

@dsnet I do not have a problem particularly. What I have is concern. In an imaginary scenario.
log/slog produces JSON logs how they SHOULDN'T look like by RFC8259.
I know, it's the user's fault, but...
That's why I suggested that it would be worth of being in std lib.
I know, I can use the 3rd party library, and I'm doing it. At least, until we fully migrate and verify that everything is working just fine.

Are there any other commonly-used JSON parsers that will fail to parse {"a":1,"a":2} by default?

Are there any other commonly-used JSON parsers that will fail to parse {"a":1,"a":2} by default?

JavaScript, the father of JSON si definitely not.
Default Python takes the second as well.
Maybe it's not a good strategy to be more Catholic than the Pope.

In "encoding/json/v2", we are aiming to reject duplicate names by default, but make it fairly easy to opt-out of the strict check.
The decision to reject duplicate names is driven by various security exploits that have occurred over the years that depended on abusing the unspecified behavior that occurs with duplicate names (across a variety of JSON implementations in various languages). See CVE-2017-12635 as one such example. This particular security stance doesn't benefit logging as much, since the content of a log message is rarely used to alter some critical piece of control flow. However, JSON is used for so much more than just logging (e.g., RPCs), that it is safer to be secure by default even if it is the more annoying default for logs analysis.
Also, even if various libraries allow for duplicate names, the behavior of how duplicates are handled varies widely. It is not enough to just ask the question of whether it is permissible. In the CVE mentioned above, Erland and JavaScript disagreed about the semantic meaning of a JSON object with duplicate names. In a similar way, permitting duplicate names, means that you may end up with logs analysis results that differ (perhaps wildly) depending on what JSON implementation you happened to use.

I think this is not just a JSON topic. The log/slog package should be explicit about the semantics of duplicate keys provided via log.Log(…) or by having duplicates via log.With(…).Log(…). With a well-defined semantics each Handler must find its strategy to implement it. Otherwise, the information in a log would depend on the choice of the log Handler – not on what was provided by the application that uses the logger.

I stumbled onto this problem while I was trying to add some slog/zerolog handlers to a forked copy of go-logging-benchmarks. I filed a ticket there because half of the systems they're benchmarking duplicate a lot of test attributes (and the other half don't) and I felt like the benchmarks might not be completely accurate. Now I'm working on a suite of slog.Handler tests, with the primary motivation to check for duplicate fields, as slogtest does not.
My personal feeling is that duplicate fields in JSON are bad (many arguments above) and should be documented as such.
The simplest solution is #2 latest, though there may be unpredictability in "latest" in some situations. Or perhaps, if field "alpha" repeats, turn it into an array of all the values.
In any event, as mentioned earlier, the log/slog documentation should clarify the expected behavior, whatever it is supposed to be, sooner rather than later.

Perhaps a somewhat different problem, but my use-case required a need for merging / overlaying a new set of slog.Attr over the ones already stored in  slog.Logger.
Considering a log/slog output like:
{
  "time": "2009-11-10T23:00:00Z",
  "level": "INFO",
  "msg": "hello_world",
  "uuid": "3a48d92f-8bf9-4633-8d35-682618412afa",
  "runtime": {
    "lifecycle": {
      "retry": {
        "total": 4,
        "timeout_seconds": 10,
        "attempt": 1
      }
    }
  },
  "runtime": {
    "lifecycle": {
      "retry": {
        "attempt": 2
      },
      "duration": {
        "nanoseconds": 123456
      }
    }
  },
  "foo": "inline_attr"
}

A potential merge would overlay the updated slog.LogValue over the original set, without modifying any existing keys of slog.Attr(s).
{
  "time": "2009-11-10T23:00:00Z",
  "level": "INFO",
  "msg": "hello_world",
  "uuid": "3a48d92f-8bf9-4633-8d35-682618412afa",
  "runtime": {
    "lifecycle": {
      "retry": {
        "total": 4,
        "attempt": 2,
        "timeout_seconds": 10
      },
      "duration": {
        "nanoseconds": 123456
      }
    }
  },
  "throughput_bps": 59
}

I also experimented with implementing slog.LogValuer on a type Runtime struct {}, adding it as a root attribute.  I felt that was too expensive and unnecessary to render everytime. I'd be keen to explore better ways of achieving the same result if folks here have ideas.
I've created a Handler (https://github.com/snqk/slog-meld) based on this approach, in case anyone is looking for something similar. I'd appreciate any constructive feedback!

I haven't looked at the code in detail, but it seems reasonable.
I would test your handler with testing/slogtest.

I think that deduplicate is expensive,  that's not what a log formater is for.
The slog.Handler is an interface, your custom JSONHandler is good enough for it. So let's keep the default one as fast as possible.

In a previous post on this issue I was pushing for no duplicates in logged JSON. Having spent some quality time with various log/slog handlers and written one myself I now feel like removing duplicates is a fairly costly endeavor and should be optional.
I have compared slog.JSONHandler to snqk/meld and veqryn/slog-dedup (both wrapped around slog.JSONHandler) using my log/slog handler test harness:

https://madkins23.github.io/go-slog/scores/:Dedup/summary.html

Both of them work fine (or as well as the wrapped handlers, anyway). Note the performance cost in both cases.
FYI: @snqk @veqryn

https://madkins23.github.io/go-slog/scores/:Dedup/summary.html

Thanks @madkins23
Overall I'd agree that merging/deduplication should not be offered as default. It's also important to note that deduplication ⊂ merge.
However I disagree that merging is that expensive. The benchmarks you posted show the meld logger isn't significantly behind the default logger (~97 vs ~87).

Expensive is subjective. My goal is to provide data that will help developers choose the appropriate solution for the requirements of a particular project. Your handler is a useful and reasonably efficient option. Thanks for contributing!

I had to change the URL above to:

https://madkins23.github.io/go-slog/scores/+Dedup/summary.html

due to limitations on file names in sum.golang.org/lookup.

@snqk: The "score" numbers on that URL are a somewhat misleading. They are rolled up using an algorithm I pulled out of my posterior. Much useful detail is lost. :disappointed:
I added tables to the score page showing the actual time and memory numbers from the benchmark testing. Check the score page for dedup again and use the Macros, Allocs, and Bytes buttons to the upper right of the score table to see this additional detail.

@madkins23: aye, I'm not sure how the score was calculated but it was easier to reference :smile:. I did look at the rest of the benchmarks though.
While all features come at some cost, your benchmarks helped me discover a couple performance related bugs (thanks!). Not as straightforward to fix, but I believe that's what's causing the bloated numbers for some benchmarks, notably BigGroup.
Going back to the OP though, I don't think this needs to be offered as a feature of JSONHandler. Apart from the obvious performance cost, this problem applies equally to TextHandler as well which isn't considered (as others have previously pointed out).
Outside of convenience, I don't hold a strong opinion on this feature being offered as an independent handler in log/slog.

Glad the benchmarks helped, that's what they're for!
I agree that this doesn't need to be pushed into JSONHandler (or TextHandler for that matter). Your wrapper approach seems appropriate for cases where this feature is needed


A request - not a bug

IDK if it's possible for you to locate the code that truncates the subject line in the list of e-mail (typically above the preview pane) and change that number to at least 99 (preferably 255).  Eudora truncates the appearance of the subject line at 62 or 63 characters.  Sorry if I should have posted this via some other method on Github.

My apology for the delayed response; the email notification of this issue scrolled off the top of my Eudora inbox and I neglected to look back until now.
Unfortunately the 64-character limit on the length of the subject line in the message list can't be changed. It's built into the structure of the TOC (table of contents) file, which has a fixed 64-byte field to store the subject.
The good news is that the subject stored in the message itself doesn't have that limitation, so you can see the whole thing when you view the message.

No problem.  Thanks for replying and for the technical explanation!
On Thursday, January 12, 2023 at 12:28:02 PM EST, Len Shustek<EMAIL_ADDRESS>wrote:

My apology for the delayed response; the email notification of this issue scrolled off the top of my Eudora inbox and I neglected to look back until now.
Unfortunately the 64-character limit on the length of the subject line in the message list can't be changed. It's built into the structure of the TOC (table of contents) file, which has a fixed 64-byte field to store the subject.
The good news is that the subject stored in the message itself doesn't have that limitation, so you can see the whole thing when you view the message.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: @.***>


Type 'ImageDecoderCallback' not found.

1.0.10.
Running Gradle task 'assembleDebug'...
../../.pub-cache/hosted/pub.dev/crop_image-1.0.10/lib/src/crop_controller.dart:352:55: Error: Type 'ImageDecoderCallback' not found.
ImageStreamCompleter loadImage(UiImageProvider key, ImageDecoderCallback decode) => OneFrameImageStreamCompleter(_loadAsync(key));
^^^^^^^^^^^^^^^^^^^^
../../.pub-cache/hosted/pub.dev/crop_image-1.0.10/lib/src/crop_controller.dart:352:55: Error: 'ImageDecoderCallback' isn't a type.
ImageStreamCompleter loadImage(UiImageProvider key, ImageDecoderCallback decode) => OneFrameImageStreamCompleter(_loadAsync(key));
^^^^^^^^^^^^^^^^^^^^

I can't reproducce this, it finds it for me. Which Flutter version do you use?

I use  => ImageStreamCompleter load(UiImageProvider key,DecoderCallback callback)
replace => ImageStreamCompleter loadImage(UiImageProvider key, ImageDecoderCallback decode)
then it works

You probably use a very old Flutter SDK then. The old approach before ImageDecoderCallback was deprecated in 3.7.0. We're at 3.10.6. now. Please, upgrade it -- as any other actively maintained package or plugin, we follow the development and changes of the Flutter SDK, too.

=>
Flutter 3.7.9 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 62bd79521d (4 months ago) • 2023-03-30 10:59:36 -0700
Engine • revision ec975089ac
Tools • Dart 2.19.6 • DevTools 2.20.1

Yes, that's a problem. We're at 3.10 now, not 3.7. You haven't upgraded your Flutter in quite some time -- in 4 months, actually, as it shows.

Ive fixed this
can you please accept this pull request


Pull request not triggering workflow

Im using the below actions in one of the workflow
  - name: Create PR
    #if: startsWith(github.ref, 'refs/heads/feature')
    uses<EMAIL_ADDRESS>    with:
      github_token: ${{ secrets.GITHUB_TOKEN }}
      title: ${{ github.event.commits[0].message }}
      assignee: ${{ github.actor }}
      label: automatic,feature
      # template: .github/PULL_REQUEST_TEMPLATE/FEATURE.md
      old_string: "**Write you description here**"
      new_string: ${{ github.event.commits[0].message }}
      get_diff: true
      target_branch: main

in the same repo i have another workflow which triggers when the pull request is created to main
  name: secondworflow.yaml
   on:
    push:
      branches: [ main ]
    pull_request_target:
      types: [assigned, opened, synchronize, reopened]
      branches:
        - main
        - 'release/**'
    workflow_dispatch:

  permissions:
    id-token: write
    contents: read
    issues: write
    pull-requests: write

  jobs:
    build:
      *********

manually creating pull request to main is triggering the seconfworflow, but the PR's which are created using this action is not triggering,

Hello @vinayprakash893
It's actually a feature of GitHub itself https://docs.github.com/en/enterprise-server@2.22/actions/security-guides/automatic-token-authentication#using-the-github_token-in-a-workflow:~:text=When you use,push events occur.
You would need a different token to make it work solely with GitHub Actions.

Thank you @ChristophShyper , i have changed to use with PAT tokens from my ID
github_token: ${{ secrets.REPO_SCOPED_TOKEN }}


feat: modular interface tests

This PR updates js-ipfs-api to use the new interface-ipfs-core tests. This gives us fine grained control over what tests we run.
Please see the description on the PR for more information on the reasons behind this change.
This also enables a bunch of tests that weren't previously being run 🎉 ...and removes a LOT of duplicated code for running the interface tests.
requires https://github.com/ipfs/interface-ipfs-core/pull/290

y no green CI?

@diasdavid I had this all lined up to be merged asap after<EMAIL_ADDRESS>was released.
After 0.70 was released I saw that there were many issues on CI that didn't appear at all when run locally.
Jenkins had a bad day last week coinciding with the release so it's been difficult and slow to debug and resolve the issues. I now have a PR open on interface-ipfs-core to resolve them https://github.com/ipfs/interface-ipfs-core/pull/319 (🤞 hopefully all of them). It would be rad if you have time to take a quick look.
I'm super anxious to get this and https://github.com/ipfs/js-ipfs/pull/1389 merged asap because they're both depending on 0.69.x still. It's confusing and frustrating the community when trying to add tests to interface-ipfs-core right now 😞

FYI @diasdavid the 2 DAG tests that are failing here will be fixed by this PR https://github.com/ipfs/js-ipfs-api/pull/801

@diasdavid gr gr gr greeeeeeen!


Sunburst Chart Support

Something like:
https://echarts.apache.org/examples/en/editor.html?c=sunburst-visualMap
Or
https://echarts.apache.org/examples/en/editor.html?c=sunburst-borderRadius
With labels on click or hover

Closing with: https://github.com/mit-cave/cave_static/commit/d4a935ebcec4df7a96f786d757667de574966b96


MSID: 92175 Version: 1 DOI: 2<IP_ADDRESS>9560

"msas": "Neuroscience"
"msid": "92175"
"preprintDoi": "10.1101/2<IP_ADDRESS>9560"
"Reviewed Preprint posted": "2023-11-09"
"Sent for peer review": "2023-08-28"
"Posted to bioRxiv": "2023-09-27" (link: "Go to bioRxiv": "https://www.biorxiv.org/content/10.1101/2<IP_ADDRESS>9560v1/")
[PLACE PDF URL HERE WHEN AVAILABLE] See step 7
Step 1. Inform bioRxiv
Who can help: @QueenKraken, @nlisgo, @scottaubrey

[x] Manuscript is in data hub index (https://data-hub-api.elifesciences.org/enhanced-preprints/docmaps/v2/by-publisher/elife/get-by-manuscript-id?manuscript_id=92175)

Step 2. Create preview of manuscript
Who can help: @fred-atherden, @nlisgo, @scottaubrey

[ ] Manuscript is available for preview (https://prod--epp.elifesciences.org/preview/92175/v1)

[ ] Trigger import for staging: https://prod--epp.elifesciences.org/import
[ ] Sync data folder with s3://prod-elife-epp-data/data
[ ] Trigger import for staging: https://staging--epp.elifesciences.org/import
[ ] Sync data folder with s3://staging-elife-epp-data/data
[ ] Pull request created to enhanced-preprint-data
[ ] Post link to pull request below

Pull request: [PLACE LINK TO PULL REQUEST HERE]
Detailed instructions: https://github.com/elifesciences/enhanced-preprints-data#add-a-manuscript
Step 3: Awaiting public reviews and QC
Who can help: Production team

[ ] QC passed or QC not needed

Who can help: Editorial team

[x] Public reviews available on sciety (https://sciety.org/articles/activity/10.1101/2<IP_ADDRESS>9560)
[x] Manuscript and editor details at the top of this issue (Supplied by Editorial team)
[ ] Reviews returned by EPP - https://prod--epp.elifesciences.org/api/reviewed-preprints/92175/v1/reviews

Example
"msas": "Genetics and Genomics", "Neuroscience"
"msid": "84628"
"version": "1"
"preprintDoi": "10.1101/2<IP_ADDRESS>4241"
"articleType": "Reviewed Preprint"
"status": "Published from the original preprint after peer review and assessment by eLife."

"Reviewed Preprint posted": "2023-01-02"
"Sent for peer review": "2022-10-28"
"Posted to bioRxiv": "2022-11-21" (link: "Go to bioRxiv": "https://www.biorxiv.org/content/10.1101/2<IP_ADDRESS>4241v1")

Editors:
Reviewing Editor
Michael B Eisen
University of California, Berkeley, United States
Senior Editor
Michael B Eisen
University of California, Berkeley, United States

Step 4: Modify manuscripts.json (no PDF)

[ ] Reviewed preprint is published to EPP (https://elifesciences.org/reviewed-preprints/92175)

[ ] Pull request created to enhanced-preprint-client
[ ] Pull request switched to "Ready for review"
[ ] Post link to pull request below

[ ] Request that a doi be registered

[ ] Post link to #enhanced-preprint comment below

Pull request: [PLACE LINK TO PULL REQUEST HERE]
#enhanced-preprint comment thread: [PLACE LINK TO COMMENT HERE]

Instructions to modify manuscripts.json

Visit: https://github.com/elifesciences/enhanced-preprints-client/actions/workflows/publish-manuscript.yaml
Click: Run workflow
Complete the form and click "Run workflow"
A successful run should result in a new pull request at https://github.com/elifesciences/enhanced-preprints-client/pulls
Open the pull request and click the "Ready for review" button to trigger tests
Once the tests pass and you are happy with the changes the PR can be merged

Example pull request: https://github.com/elifesciences/enhanced-preprints-client/pull/334/files
Once the pull request is merged in it should be available a few minutes later.

Request that a doi
Post the following in #enhanced-preprint:
@Fred can you register a doi for https://elifesciences.org/reviewed-preprints/92175

Step 5: Awaiting search reindex

[ ] Reviewed preprint is avaliable on journal homepage (https://elifesciences.org)

The search reindex is triggered once an hour. We need the reviewed preprint to be indexed as the search application serves the journal homepage.

Additional info
If needed, the jenkins pipeline to reindex search can be triggered sooner.
https://alfred.elifesciences.org/job/process/job/process-reindex-reviewed-preprints/

Step 6: Published! Request PDF generation

[ ] Post the link to the PDF url at the top of the issue

[ ] Post link to #enhanced-preprint comment below

#enhanced-preprint comment thread: [PLACE LINK TO COMMENT HERE]
Post the following to the #enhanced-preprint on slack:
@Ryan Dix-Peek please can you generate a PDF for https://elifesciences.org/reviewed-preprints/92175

Step 7: Introduce PDF to data folder and git repo

[ ] PDF is avaliable at https://github.com/elifesciences/enhanced-preprints-data/raw/master/data/92175/v1/92175-v1.pdf

Detailed instructions: https://github.com/elifesciences/enhanced-preprints-data#add-a-pdf
Step 8: Add PDF url to manuscripts.json

[ ] Reviewed preprint PDF is available for download (https://prod--epp.elifesciences.org/reviewed-preprints/92175)

[ ] Pull request created to enhanced-preprint-client
[ ] Pull request switched to "Ready for review"
[ ] Post link to pull request below

[PLACE LINK TO PULL REQUEST HERE]

Instructions to add PDF url to manuscripts.json

Visit: https://github.com/elifesciences/enhanced-preprints-client/actions/workflows/add-pdf-url-to-manuscript.yaml
Click: Run workflow
Complete the form and click "Run workflow"
A successful run should result in a new pull request at https://github.com/elifesciences/enhanced-preprints-client/pulls
Open the pull request and click the "Ready for review" button to trigger tests
Once the tests pass and you are happy with the changes the PR can be merged

Example pull request: https://github.com/elifesciences/enhanced-preprints-client/pull/397/files
Once the pull request is merged in it should be available a few minutes later.

Step 9: Done!

[ ] Kettle is on!

elocations missing from references.


MapR Warden permissions fix

We have found warden.conf is often set to root and this breaks the cluster coming up in our environments (probably where the base OS's UMASK is set more restrictive than yours as the file ends up with root:root 0600 permissions).
By setting warden.conf to be owned by the mapr user this seems to fix this and allows the warden process to start and bring up the cluster.

do you have umask 022?
somehow this pull request has to many commits in it, not only the described changed.

umask is 027 and yeah but all 3 patches should be merged anyway. I can re-submit individually but it hardly seems worth it given all 3 should be integrated.

Replaced with pull request #33


install dep plugins & run provider hooks the same as shell hooks

Still need to add tests and open to changing how dep plugins get installed. Currently it waits until all deps are installed and then scans all dep config files for plugins listed and installs them at that point. Pretty inefficient.

So does it still install plugins first locally, and then again for deps?

This seems okay so far, as long as lower-level plugins don't override top-level ones.

Just noticed there already is a provider hook test and it continues to pass.
Right, global and local plugins are installed first.

Added the resource fix to this PR as well. This way will be able to be changed to work without hooks due to the changes in this PR https://github.com/kellymclaughlin/rebar3-tidy-deps-plugin


Underscore in description breaks rendering

Following attributes:
+ is_good: false (boolean) - Foo bar.
+ is_bad: false (boolean) - Foo_bar.
+ is_good_2: false (boolean) - Foo_bar.

Render like this:
{
        ...
        "is_good": false,
        "is_bad: false (boolean) - Foo_bar.": null,
        "is_good_2": false,
        ...
}

Screenshot:
https://www.dropbox.com/s/jgjvjxdevopy1y0/Screenshot 2015-05-08 13.28.45.png?dl=0

Hey @pzurek
Since _ is a reserved character according to MSON spec, we try to parse it to our best. The above result is correct. But I think the following is what you want to do:
+ is_good: false (boolean) - Foo bar.
+ `is_bad`: false (boolean) - Foo_bar.
+ is_good_2: false (boolean) - Foo_bar.

The problem is that it behaves inconsistently. If you have an odd number of underscores on a line it looks ok, if you have an even number it breaks. In my opinion it should behave always the same. But I'm glad there's a workaround, although I'm not sure it will cover all possible cases. I'll come back to comment here if I discover another case that breaks it.
Thanks for a quick response on this @pksunkara.

@pzurek Yes, I understand what you mean by even and odd inconsistency. We wanted to ask users to escape the property name whenever there is a _ in it, but since it is very inconvenient, we decided to be okay with even _ in the property name (treating it as an inclusive escape character).

@pzurek I believe the problem is related to Markdown's use underscore _ to mark text in italics – when there is an odd count of underscore it is OK but even number is matched and everything in between is treated as one span block..
E.g.
Source: _this will be italicized_ vs. _this won't
Render: this will be italicized vs. _this won't
TL;DR: Markdown syntax

@zdne True about the Markdown syntax.


Include mock globals

Closes: https://github.com/GoogleCloudPlatform/google-cloud-php/issues/654
@stanley-cheung, please let me know if this resolves the issue for you! :)

Touched base with Stanley offline, this is good to merge.

Verified the fix. Thanks!


Fixed exporting types

Exporting types before creating a table that will be returned from a module, has some weird effects sometimes (I actually don't know what's causing this bug).
This simple change makes it so the types are reachable/usable with Roblox LSP for VSCode.

Huh, that's unintuitive. Fair enough.


"Step into (F11)" doesn't seem to work in Visual Studio when debugging through F# Interactive

I was experimenting with the new F# Interactive debugging feature in Visual Studio 2015 RC and found out that "Step into" (as well as the other buttons next to it) doesn't seem to work correctly. Instead of stopping on the next line, they simply resume program execution (as if I had clicked on "Continue", instead).
This behavior is not observed when debugging a standard F# executable, which seems to work fine.

F# interactive generates optimized code by default, which leads to a fairly poor debugging experience.  You might just be seeing that.
If you check the F# Interactive options, is the top script debugging setting marked "true"?  "true" will enable debug-friendly options for F# interactive (you'll need to reset the session, though).

My current settings are:

I've tried also removing the "--optimize", as it could be changing in strange ways the generated code but it was of no avail. The previously described behavior still persists.

Then this is likely just some kind of code pattern that has poor sequence-point generation.
Can you share your code and the exact debugging steps you tried?

Sure:

Added x() definition to the FSI (ALT + ENTER), then started F# Interactive debugging session, then executed x(). The debugging session starts and stops in the breakpoint. I then:

It just resumes execution and doesn't stop on the next line.

Hmm, I don't repro with the latest builds here.

Do you have "Enable Just My Code" enabled in the debugger?

Yes, I had. I disabled it and now it works perfectly! That was it.

This is unrelated to the issue, but that GIF video is really impressive. Lincoln, may I know how you created that?

Screen2Gif - http://screentogif.codeplex.com/

Thank you! I especially like how smooth the rendering is.


ci support "run performance"

Proposed changes
Issue Number: close #xxx

Further comments
If this is a relatively large or complex change, kick off the discussion at<EMAIL_ADDRESS>by explaining why you chose the solution you did and what alternatives you considered, etc...

run buildall

run external

run pipelinex_p0

run buildall


Update service-bus-async-messaging.md

Removed repetitive bullet points.

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@brianharwell : Thanks for your contribution to the Azure documentation! The author, @sethmanheim, has been notified to review your proposed change.

@brianharwell Thank you for catching that! We appreciate the feedback and will get that incorporated right away. #sign-off

Glad to help! I think being able to provide feedback and corrections is fantastic!

@brianharwell And we really do appreciate it. Thanks again!


Update image

Thought it would be good to update the default value of image version as many improvements have been made since 0.16.0.
https://github.com/oliver006/redis_exporter/releases

/ok-to-test

/assign
/lgtm

/check-dco


Was not declared in this scope.

Now Following problem is coming up.
I hope you can help me out.
I double checked the follow items for installation.
Install the following libraries from the Arduino IDE Library Manager: ESP8266WebServer ESP8266WiFi ArduinoJson WiFiManager NTPClient IRremoteESP8266 as well as Cryptosuite which is not on the IDE
All the libraries are the latest version.
Was not declared in this scope:
case SANYO_LC7461: output = "SANYO_LC7461";       break;
case DENON:        output = "DENON";              break;
case COOLIX:       output = "COOLIX";             break;
Class decode_results Has no member named 'repeat'
if (results->repeat) Serial.print(" (Repeat)");
Thanks a lot for support.
Lampenschakelen

This looks to be a problem with your IRRemote Library.  I would reinstall or verify your installation or that. I see changes regarding the repeat parameter.
Sent from my iPhone

On Dec 6, 2017, at 16:43, lampenschakelen<EMAIL_ADDRESS>wrote:
Now Following problem is coming up.
I hope you can help me out.
I double checked the follow items for installation.
Install the following libraries from the Arduino IDE Library Manager: ESP8266WebServer ESP8266WiFi ArduinoJson WiFiManager NTPClient IRremoteESP8266 as well as Cryptosuite which is not on the IDE
All the libraries are the latest version.
Was not declared in this scope:
case SANYO_LC7461: output = "SANYO_LC7461"; break;
case DENON: output = "DENON"; break;
case COOLIX: output = "COOLIX"; break;
Class decode_results Has no member named 'repeat'
if (results->repeat) Serial.print(" (Repeat)");
Thanks a lot for support.
Lampenschakelen
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

This fix the problem with the IR CODES
I did a new installation of Library IRRemote.
New problem is coming up
Arduino: 1.8.5 (Windows), Board: "NodeMCU 1.0 (ESP-12E Module), 80 MHz, 115200, 4M (3M SPIFFS)"
C:\Users*\Documents\ESP8266-HTTP-IR-Blaster-master\ESP8266-HTTP-IR-Blaster-master\IRController\IRController.ino: In function 'void cvrtCode(Code&, decode_results)':
IRController:1080: error: 'RAWTICK' was not declared in this scope
   r += results->rawbuf[i] * RAWTICK;

                             ^

C:\Users*\Documents\ESP8266-HTTP-IR-Blaster-master\ESP8266-HTTP-IR-Blaster-master\IRController\IRController.ino: In function 'void dumpRaw(decode_results)':
IRController:1129: error: 'RAWTICK' was not declared in this scope
 uint32_t x = results->rawbuf[i] * RAWTICK;

                                   ^

C:\Users*\Documents\ESP8266-HTTP-IR-Blaster-master\ESP8266-HTTP-IR-Blaster-master\IRController\IRController.ino: In function 'void dumpCode(decode_results)':
IRController:1162: error: 'RAWTICK' was not declared in this scope
 Serial.print(results->rawbuf[i] * RAWTICK, DEC);

                                   ^

Multiple libraries were found for "IRremoteESP8266.h"
Used: C:\Users*\Documents\Arduino\libraries\IRremoteESP8266
Not used: C:\Users*\Documents\Arduino\libraries\arduino_244590
exit status 1
'RAWTICK' was not declared in this scope
This report would have more information with
"Show verbose output during compilation"
option enabled in File -> Preferences.

How are you installing IRRemote? This is another error coming from the IRRemote library that doesn’t make sense
Sent from my iPhone

On Dec 6, 2017, at 18:50, lampenschakelen<EMAIL_ADDRESS>wrote:
This fix the problem with the IR CODES
I did a new installation of Library IRRemote.
New problem is coming up
Arduino: 1.8.5 (Windows), Board: "NodeMCU 1.0 (ESP-12E Module), 80 MHz, 115200, 4M (3M SPIFFS)"
C:\Users*\Documents\ESP8266-HTTP-IR-Blaster-master\ESP8266-HTTP-IR-Blaster-master\IRController\IRController.ino: In function 'void cvrtCode(Code&, decode_results)':
IRController:1080: error: 'RAWTICK' was not declared in this scope
r += results->rawbuf[i] * RAWTICK;
                         ^

C:\Users*\Documents\ESP8266-HTTP-IR-Blaster-master\ESP8266-HTTP-IR-Blaster-master\IRController\IRController.ino: In function 'void dumpRaw(decode_results)':
IRController:1129: error: 'RAWTICK' was not declared in this scope
uint32_t x = results->rawbuf[i] * RAWTICK;
                               ^

C:\Users*\Documents\ESP8266-HTTP-IR-Blaster-master\ESP8266-HTTP-IR-Blaster-master\IRController\IRController.ino: In function 'void dumpCode(decode_results)':
IRController:1162: error: 'RAWTICK' was not declared in this scope
Serial.print(results->rawbuf[i] * RAWTICK, DEC);
                               ^

Multiple libraries were found for "IRremoteESP8266.h"
Used: C:\Users*\Documents\Arduino\libraries\IRremoteESP8266
Not used: C:\Users*\Documents\Arduino\libraries\arduino_244590
exit status 1
'RAWTICK' was not declared in this scope
This report would have more information with
"Show verbose output during compilation"
option enabled in File -> Preferences.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

Removed IRremoteESP8266
Did fresh new installation
Include Library -->  Manage libraries --> search for IRremoteESP8266
Now it's done compiling.
Finally it's working thanks so much for your good support.

Receiving IR signals is working perfectly.
Sending  ir signals is not working.
When i try change the IR transmitter for a normal led i see the led flashing.
But when i change it back for the IR led it's not received by the  wemos d2 mini with a IR receiver.
When i press remote control the led is flashing that i'ts receiving.
I double check the resistors and all the cables every thing is connect to number D2.
I hope you can help me out

is used the picture above.

Check and see if the LED works with a camera, most phone cameras can detect IR (iPhone front camera works, rear camera does not, has an IR filter)
Also send a picture of your actual build and let me see if its assembled correctly; possible that you reverse the poles on your IR LED?

Also are you transmitting and receiving to the same device or 2 different devices? Its designed to ignore its own signal

I test it 2 different devices.
Also with front camera  i see very small light flash..
I think the IR transmitter gets to less power.  I used everything in you schematic

Send a picture of t he build let me verify
It should get plenty of power. What resistor are you using?

Signal of the esp8266 IR transmitter is weaker that that of a remote control.  When i do the test with a front camera. When i do the testing with a wemos d2 to receive the IR singals It's not receiving. When i press the button of my remote control it's receiving the IR singals without any problems.  I think that's why my amplifier is not responding because the trasmitter IR singal is not strong enough.

So it looks to me like you might have your resistors switched though its very difficult to tell since I'm not sure which pictures correlate with which measurement
The low end 100 ohm (or even less, I use 10 ohm) resistor needs to be in series with the LED and the higher resistance one (I use 1000 ohms) should be connecting the data pin D2 and the middle pin of the 2N2222 transistor
If you switch those it will severely dampen the brightness of your transmitter
Those resistor values may vary from LED to LED though; I use an ultrabright IR LED and my range is pretty good, about 20 ft

Also curious what signal you're trying to send that's not working
Is it a normal encoded signal or a RAW or complicated signal like DENON etc

100 ohm resistor is not conected to led

Well it should be and that would certainly explain the brightness issue

I do not use node red, no
Hadn't really heard of it to be honest
But glad you got everything working

Thanks it's working perfect with NodeRed ;-)


Added Mark example notebooks using pyplot

Added two new folders in the Marks folder: Object Model and Pyplot
Moved the existing notebooks to 'Object Model' folder
Added pyplot examples to the Pyplot folder
Updated the Index notebook to provide the links to both the Object Model and Pyplot implementations

@rmenegaux can you please review?


Proposal for async-blocks for web requests and shell execution

Multiple issues have been opened requesting specific libraries or functions to be ported/included (#160, #161, #186, #184, #233). On one hand, waiting for these to be ported might take a while and hinder adoption in the meantime. On the other, one might ask whether it is even worth the effort to port everything to typst directly if those libraries offer vector export natively.
I think a solution to this problem might be to allow the execution external of binaries and web requests. People using typst offline would then be able to use python/julia (#117, #244) and their favorite tools, as well as allow for loading data directly from databases or APIs.
With the steady improvements to webAssembly and projects such like pyodide and wasmer binaries could even be packaged as wasm-modules allowing the use offline as well as online. If a consensus for some kind of built-standardization of those wasm-modules could be reached, one could even imagine a some kind of package manager, which automatically gets the latest version of some library and compiles to wasm.
For this to work in practice I propose for typst to support an async-code-block:
#async {
	...
}

Everything inside an async-block is private in scope and gets deleted after being run. Outside variables are read-only. Async-blocks only get run when manually triggered by the user or with a command line flag. That way outside binaries or web requests don't get triggered constantly while editing or without the users knowledge for security reasons.
If one wants to use the result of a web request or an external binary, it has to be saved to a file. This file can then be read normally outside the async-block. Forcing the user to save the data to files, would allow the document to still be parsed correctly when offline and make it easy to be kept for archival purposes.
This would (I think) still be within the core principles of typst:

In Typst, all functions are pure. This means that for the same arguments, they always return the same result. They cannot "remember" things to produce another value when they are called a second time.

and

Performance through Incrementality: All Typst language features must accommodate for incremental compilation. Luckily we have comemo, a system for incremental compilation which does most of the hard work in the background.

Since everything outside the async-blocks would use read/image to load created files, there wouldn't be any change. Async-blocks themselves would also follow this principle, with the only difference that they only get run when triggered.
I imagine these kind of functions could be supported inside async-blocks:
#async {
	/* Send a post request to a url */
	post(
		url,
		payload,
		...
	)

	/* Send a get request to a url */
	get(
		url,
		params,
		...
	)

	/* Expose curl for more complicated use-cases */
	curl(
		...
	)

	/* Execute a command with arguments */
	command(
		path,
		arguments
		...
	)

	/* Create or override a file with content */
	save(
		content,
		file
	)
}

Let's take mermaid.js as an example of how this could look like in practice:
A simple typst Mermaid-Library:
#let generate(code, file) = {
	async {
		let diagram = command('mermaid', code)
		save(file, diagram)
	}
}

The folder would also contain mermaid binaries for linux, mac, windows and/or wasm.
Document:
#import "library/mermaid/mermaid.typ"

#let diagram = [
graph TD;
      A-->B;
      A-->C;
      B-->D;
      C-->D;
]

#mermaid.generate(diagram, 'images/diagram.svg')

#image("images/diagram.svg", width: 80%)

There are probably better ways to approach this, but after thinking about it for a day, I would like to know what others would think of such an approach.

@4cq2 I agree that this is a very broad issue, but I think they are in principle linked. The only reason for an async-block are command-execution or web requests, and I don't see how those could be implemented without some kind of special execution if the basic design principles are not to be broken. This is just my proposal for how the special execution could work.

A bit of a SPAM as I already posted this exact answer on another issue, but I feel like re-iterating:
I am just here to comment regarding the idea of a --shell-escape flag. (please don't). There are three main reasons why that is not desirable:

it would be a giant security hole (especially once the package manager arrives)
it would lead to non-deterministic rendering (different version of external CLI tools producing different outputs)
it would lead to a fracturing of the ecosystem between windows, linux and macos (I can't wait to have to do if platform() == "windows" in my text document)

Additionally, as previously mentioned, Typst does not need to be a generic language, if features that cannot be created using the default language exist (which they probably do), I believe that there should be a plugin API (most likely using WASM as it is a decent encapsulation of third party code with near-native performance). It fixes all of the aforementioned issues.
Additionally, looking at the tectonic project, they did not want to add a --shell-escape flag until the community was really bullying them into doing so. I think that the LaTeX ecosystem would be better without this feature, and I strongly believe that, in the long term, this will be better for the Typst ecosystem as well.

We can have more focused issues for separate things, but this is indeed overly broad.


add setup instructions for other operating systems

This will be postponed until further notice due to the lack of compatibility with @cmyui's library and windows.

Ah yea this could take a while, simply put we just have many other priorities to get working before we go for pure usability stuff.
Windows support will likely come once the server is usable in production environments. ;)

Docker container?