New Rule: No functions with less than X usages.

Expected Behavior of the rule
The core feature needed for this rule would be the ability to count the number of usages of a member. For a "prototype" of this rule, Detekt would need to scan the entire source set that the member belongs to. For a fully functional version of this rule, Detekt would also need to be able to scan across modules and count usages from other modules.
The user can simply configure a threshold X for the maximum(or minimum) number of usages per function (or class, val, var, etc.).
The user should also be able to configure whether or not to include test source sets in the usage counting.
Context
Essentially, this is meant to provide a similar feature to the IDE highlighting of unused members. IntelliJ currently has an issue that causes test sources to be included in usage-counting for kotlin (in contrast to Java, which has the ability not to include test sources). Even if IntelliJ did not have this issue, a Detekt rule would still provide additional benefits beyond what IDE highlighting can provide, namely automatic checks and reports and better portability.
Unused members are code smell because they increase the size and complexity of a project without providing any features. At worst, an unused member is indicative of a bug. At best, it is an idea for something that may be used in the future or has some other practical reason for existing. But most often, it is just old code that can be considered clutter and should be deleted or refactored.

This won't be easy to implement as detekt doesn't have the capability to scan the entire codebase and keep a context. Every rule are effectively an implementation of the visitor pattern implemented on the node of the AST.
We could extend detekt to start collecting those aggregated metrics (i.e. # of function usages vs definitions), but this is not in the roadmap

I think the question is more for @detekt/maintainers if we ever want to implement such capability or not

In my opinion, this feature might be better suited for a dedicated compiler plugin.
Technically it's possible to implement this with detekt. However, it will require more effort than just implementing a new rule. One needs to gather the information from BindingContext or from some other Kotlin compiler APIs.

Agree, detekt doesn't have the capabilities. Even if we implemented all the tracking and collation necessary for this, we would be missing out on usages from Java (and Scala/Groovy/Clojure/etc.).

You can however, write an Android Lint check for this, and use it in non-Android projects too via com.android.lint standalone plugin (see docs). That at least covers Java and .class-file usages. You can have a look at UnusedIds and UnusedResources for inspiration.

Won’t implement for the reasons listed above


Drop Internet Explorer in Vue 3 as Microsoft says bye bye to IE & Edge Legacy

What problem does this feature solve?
Vue 3 is planned to have IE 11 compatible build... but after this announcement I think this should be avoided to avoid any future bottlenecks? Your thoughts?
https://www.techrepublic.com/article/microsoft-this-is-when-ie11-and-legacy-edge-support-ends/
What does the proposed API look like?
Leaner & Modern Vue.js Framework

What's the status of this?

It's as laid out here: https://github.com/vuejs/vue-next/releases/tag/v3.0.0

Many large US companies have legacy ActiveX components which require IE11,
and which are not going to be replaced anytime soon.
So, given the choice of a single browser to support, they choose IE11.

Hello! IE11 build was supposed to have some attention in Q4. Do you guys have any news to share? :)

Do you know when we will have ie11 support ?

Hi,
I need to have an understanding what is happening with the status of Vue3 supporting IE11. As mentioned above it was due to be worked on in Q4 2020 but I am not seeing any traction. The answer will determine whether I should wait for Vue3 to support IE11 or think about another approach.
Can anyone clarify?

Hi all,
I am also in need of an expected ETA on the IE11 support. For the exact reasons mentioned above, we have a need to cater for certain user groups in such organisations and institutions, so despite the recent(ish) announcements from Microsoft we still need to have this base covered.
We are (possibly a little too eagerly) already in progress with a new large scale build based on Vue 3. We made the choice based on the significant improvements it brings to an already incredible framework and that the required lifetime of the project would mean it made more sense to start with the latest version rather than starting out life with an 'old' one.
We have been banking a little on the hope that the references to coming support in the roadmap and other tickets and articles pointing to the fact that it was soon on its way some time ago... moving us to think it was worth the risk, but honestly I'm starting to get nervous about our decision.
We are at a cross roads meaning that if we can't expect it in the coming months, we will need to downgrade and re-write to support Vue 2... to cater for this small, but significant slice of the user base, but in so losing the many benefits Vue 3 has to offer.
@yyx990803 it would really help our team out if we could understand when support is likely to be delivered so we can make the best informed decision.
I appreciate the original advice was to wait, but we took a bit of a gamble... and we might end up in a tight spot if we wait much longer for news.
We love the work you and your team have done with all of this, and I feel a little cheeky to ask this on an open source project... but can you tell us anything at all?
Apologies for commenting on an already closed ticket, but I can't find a better place to pose the question without causing interruption.


Add Windows 10 UWP implementation

Min Supported Version:
Windows 10.0 (Build 10240)
Known issues:

Testing Mode currently not available for MvvmCross (needs a platform wide change)
Does not support subscriptions (as UWP does not support this, yet)

Hey there! I have been adjusting the API recently so if you can update based on this new finalized API, should be better. I will start a review on your current and give you some feedback.

Hey @jamesmontemagno, thanks for your review. Sounds good, I will update the PR during the day and give you an update, when it's ready to get reviewed again!

Alright, I have updated the PR. Beside failing AppVeyor build. We have to take a look at that together, I guess...

I created a new UWP branch to look into it and will help out with me trying to get it working. Should be good now :)


RichTextLabel's append_bbcode() respects opening tags but not closing tags(v3.2.3.stable.official)

If using append_bbcode() to add any tag, only the opening tags are parsed- closing tags(IE [/center] or [/rainbow]) are added as literal text and do not affect the formatting.
The expected behavior is that both opening and closing tags would be treated identically- append_bbcode should not simply add the ending tag as text; it should result in the associated opening tag being closed and the closing tag not being visible.
Steps to reproduce: attach a script to a RichTextLabel node with the following code:
    func _ready():
        append_bbcode("[center] text")
        append_bbcode("[/center]")

result: text is centered, reads " text[/center]"
This occurs with all markers as far as I can tell.
There is a workaround which behaves as expected:
    bbcode_text = "[center] text"
    bbcode_text = bbcode_text + "[/center]"

result: text reads " text", is centered(no closing tag visible; tag closed as expected).

it should result in the associated opening tag being closed and the closing tag not being visible.

This isn't possible for performance reasons.


[Bug]: In the feature flag list, I'm struggling to comprehend the JSON and lengthy string variants.

What happened?
In the feature flag list, I'm struggling to comprehend the JSON and lengthy string variants.
The issue is, when there's a single lengthy string or JSON variant, the complete information isn't displayed when I hover over the variant with the mouse.

To Reproduce

Create a feature flag
Change variant as Json format
Input a Json content (more than 30 characters)
Come back to feature flag list, you will see the problem

Expected behavior
When I hover over the variant with the mouse, please ensure that the complete information is displayed.
Alternatively, consider giving each variant an easy-to-understand name. Display this name by itself, or alongside the variant content.
Additional context
No response
Code of Conduct

[X] I agree to follow this project's Code of Conduct

hi @cosmic-flood ,  I can fix this bug. Can you assign it to me?

Thanks @mucoban, I've assigned this issue to you 🍉


Fix street fields for us/al/jefferson

street_name was accidentally duplicated instead of having street_type as the second field


Tricksy Bulk Action

i have a tricky one for you guys, its a two part question:

i need to be able to add different types of "bulk actions" on a grid. but i want to avoid adding a button for each action. i was thinking a drop down and then a button to initiate the selected action. i havent found anywhere an example how to add a drop down to the grid toolbar at the top.
i need the ability to specify a parameter to the bulk action. for example if i want to bulk change the supplier to "Acme" i want a bulk action called "change supplier" and then you can choose to which supplier you want to change to. i can create a separate action for each supplier but you can see how that can get out of hand very fast.

thanks for any tips!

A good sample about adding the drop down is : https://github.com/volkanceylan/Serenity/wiki/SplitButton-dropdowns
Additionally, if you want to pass parameters, I'd recommend you have a dialog appear asking for the parameters before the action runs. this will let you best intercept the action and edit as needed.

@sayuga  thanks man .. ill take a look!


Various questions?

Hi,
Thanks a lot for this extremely useful implementation.
I wanted just to ask what is the ZFilter class, is it used to standardize the observed state according to the running mean and std of the observed states?
In addition, in the GAIL paper, they consider in the TRPO update a step in the direction of the gradient of the entropy. Is it considered here? I am not managing to find it in the code.
Thank you in advance.
Luca

Hi, I have the same question. What is the running state in your GAIL code? Is it some trick that helps the learning? I noticed that when replaced with a newly-initiated running state instead of the one load from the pickle file, the learned policy becomes much worse. Would appreciate it if you can clarify what role this running state plays.


Move React ppx from compiler repo, add tests

I find myself wanting to contribute to React ppx but always end up giving up because of the lack of infrastructure. By moving the file here, we can leverage the testing infrastructure and keeping it more in sync with the parser.
If this makes sense and gets merged, I will update the compiler repo to read from new file.
Hopefully this change makes sense as well from organizational perspective, as the React ppx is a syntax transformation.
Summary of changes:

Copied the React ppx file from:
https://github.com/rescript-lang/rescript-compiler/blob/4f4812aa4e1411029392f6dbcacc580e755d68d5/jscomp/syntax/reactjs_jsx_ppx.cppo.ml, and then applied these changes:

Add @raise annotations
Remove dead functions
Remove unused Ast_mapper.register call
Format file using width 120

Add reactjs_jsx_ppx.mli file
Added -reactJsx option to rescript.exe (for now I didn't added it to command -help output as I'm not sure if it should be exposed)
Adapt test runner to interpret test files .react.res and react.resi
Add a few tests

Thanks to these changes, I already found 2 bugs:

Comments at the top of the file are placed in the wrong position
<Foo.bar /> elements are not supported by new parser

@jchavarri This is great! Will take a look at your PR. Long term we want to merge the jsx ppx with the parser itself, i.e. do the syntax transform in the parser.

Thanks for the responsiveness @IwanKaramazow! Much appreciated :)

Long term we want to merge the jsx ppx with the parser itself.

I realized while working on this that there is no need to have it as a separate step now. It makes sense to do so, although maybe in mid-long term? e.g. 6-12 months. I mention that because I guess that merging the ppx with the parser would potentially break conversion between code in Reason and ReScript syntaxes? 🤔 Which is really useful while this transitions happens.
Also, I don't know if it's possible, but it'd be great to keep it as a separate module, so that tools like https://jchavarri.github.io/ppx-explorer/ that rely on a PPX-like API can keep updates with upstream easily, e.g here.

Hey Javier! It's been a while
We're gonna pause this specifically, because the React PPX might change in the future (cc @rickyvetter). Switching version without breakage would require a big syntax-level switch, which would need some consideration.

@chenglou hey! long time indeed :)
Could you elaborate, please? This PR doesn't change the ppx semantics, and it does not contain any breaking changes in the ppx code. All changes in Reactjs_jsx_ppx module are purely cosmetic (formatting, add reanalyze @raise attributes etc).
If there is some breaking change planned for the future for the ppx, this PR should actually help @rickyvetter and any other authors build and release that change, because once there are more tests, we will know exactly what cases are breaking after the change.
In other words, if this PR gets merged, nobody should notice anything, except the ppx maintainers and contributors, who would get a quality of life improvement thanks to the ppx now being part of the testing infrastructure in this repo.

Oh, I think I see what you mean. If PPX lives in compiler repo, then it's easy to configure bsconfig flags like it was done with jsx2 and 3. 😞 I didn't think of that.
Maybe the parser could offer a way to switch between syntaxes per file with an attribute, like Reason parser is doing: https://github.com/facebook/reason/pull/2605/files#diff-2f961a549953a72af3a11bb3b36ecc49R4975.
This would be helpful not only for ppx of course, but any breaking syntax change.

@chenglou I thought more about this. Because syntax is consumed as a git submodule inside compiler repo, nothing would prevent having 2 ppx modules with 2 different versions, and then adding any bsconfig configuration in compiler repo like it's been done in the past. From the compiler perspective, nothing changes either after this change, except it has to read the ppx file from another location. I'm curious to know what @bobzhang thinks as well.

@jchavarri Ricky (@rickyvetter) owns the module.
There's a  concern since we still need support the reason syntax, so if we spins off the jsx ppx, how would it affect the old reason syntax

@bobzhang There should be no impact in old Reason syntax. From the compiler perspective, this PR can be considered for all effects and purposes like moving a file from one folder to another.

As long as it does not make the main maintainer’s life more difficult, I am
fine with it.
But keep in mind this module is used by reason syntax too
Javier Chávarri<EMAIL_ADDRESS>周一下午9:03写道：

@bobzhang https://github.com/bobzhang There should be no impact in old
Reason syntax. From the compiler perspective, this PR can be considered for
all effects and purposes like moving a file from one folder to another.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rescript-lang/syntax/pull/124#issuecomment-683763959,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAFWMK52VPL4VL46JZEICSDSDONTHANCNFSM4QPX2FPQ
.
--
Regards
-- Hongbo Zhang

I created a draft PR to show how minimal the impact downstream for the compiler is: https://github.com/rescript-lang/rescript-compiler/pull/4660.

I think this move could make maintaining quite a bit cleaner but I want to clarify setup. In the near future we need to make a series of major breaking changes to the ppx and need to ship both versions to the compiler so that folks can update their ppx version independently of their compiler version. Would you propose just exposing both versions as different modules here and then doing the switch logic in the compiler based on config?

Hey @rickyvetter ! 👋

Would you propose just exposing both versions as different modules here and then doing the switch logic in the compiler based on config?

Yes, exactly. The companion PR in the compiler repo shows how the ppx module is available just like before (see ppx_entry module). I only had to change the linking order so that napkin.cmxa lib (this repo) is linked before the syntax.cmxa lib (that contains the ppx entry in compiler), which depends on it.
If there is a new version of the ppx, there can be another module added next to the one in this PR. Keeping these modules close to the testing infra might be beneficial too to guarantee the migration is smooth, and new tests can also be added from user reports. It seems the compiler still keeps the ppx version handling from bsconfig option.
The only thing that will change from previous setup is that every time there is a fix or update in the ppx, the compiler repo will have to sync the submodule version, while before the file could be just directly updated in-repo. I hope this is not a deal breaker.
Maybe I'm missing something :) Please let me know if there's anything else that I'm not accounting for 👍

Gotcha. Seems like a win to me. @chenglou you seemed hesitant at first - okay with this plan?

updated the PR with the following:

merge the changes from master
add flag -ppx to the main rescript.exe binary (as discussed with @IwanKaramazow offline)
remove ppx-related stuff from multiprinter (as jsx ppx seems very unrelated to printing really)
skip roundtrip tests for specs that have ppx defined
added some test externalWithCustomName that was failing before but now passes, I guess some recent change fixed it.

lmk what you think :)

@IwanKaramazow I think I tackled all the comments, let me know if there's anything missing :)

This is not finished since the removal of jsx ppx is not done in the compiler.
After some thoughts, I think this may make your test workflow more difficult, @rickyvetter , since now you make changes to the ppx, you need update the syntax submodule to test your new changes

@bobzhang I'll take care of it.

@bobzhang I had created a companion PR in https://github.com/rescript-lang/rescript-compiler/pull/4660/ that included the compiler changes. I didn't continue working on this because @chenglou mentioned on DM a few days after this PR was merged that the move to the PPX to the parser might be rethought / retracted because of the new jsx transform.

Let's sync up on this first.

I talked to @IwanKaramazow and these are the steps to take before this is finished:

Update syntax repo so that Reactjs_jsx_ppx becomes Reactjs_jsx_ppx_v3 again, as it will be useful later on, when new JSX ppx versions will be added.
Update https://github.com/rescript-lang/rescript-compiler/pull/4660 to with master and latest syntax, and make sure everything compiles.


ProxyPacMandatory setting

Where are the examples for the ProxyPacMandatory setting in the ProxySettings policy?
It would be nice if it could be included in the example s on this page.
It says "For more detailed examples go to https://go.microsoft.com/fwlink/?linkid=2094936" but the page at that link only shows Edge command line options and it doesn't mention ProxyPacMandatory at all. In my opinion it is a quite useless link in this context.

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

ID: 198c6f64-9e5c-f18e-8439-8d0d11d4228d
Version Independent ID: 83f32be1-063c-284c-dfdc-6b5976a5b7cc
Content: Microsoft Edge Browser Policy Documentation
Content Source: edgeenterprise/microsoft-edge-policies.md
Product: microsoft-edge
GitHub Login: @dan-wesley
Microsoft Alias: stmoody

@perhof Thanks for reaching out!  I'm checking with the team about the examples.

@perhof Here is information from our developer:

ProxyPacMandatory is just a Boolean field. If set, there must also be a PAC file set in the ProxySettings and all network traffic must either flow through that PAC file or the proxy must be bypassed via the ProxyBypassList field. If that PAC file fails to download or execute, all network traffic is blocked.

The policy documentation should be updated.  (https://docs.microsoft.com/en-us/deployedge/microsoft-edge-policies#proxysettings)
Thanks for the feedback!


Beta - Rework layout for saving and loading config

The saving and loading popup is confusing and needs to be reworked. Also the popup should close when a config was set correctly

Fixed


ModuleNotFoundError: No module named 'AppKit'

Error running on macOS Catalina (10.15.6) with python 3.8.5. ModuleNotFoundError: No module named 'AppKit'

I have the same problem.

I had to install PyObjC to fix this issue.
Try this
pip3 install -U PyObjC

I tried that, but my issue wasn't solved

I had to install PyObjC to fix this issue.
Try this
pip3 install -U PyObjC

It worked! But doesn't the project say it has no dependencies?

What is the module that comes with the system copy of Python on macOS?
playsound isn't supposed to have any dependencies - I'm thinking about the possibility of making it so that playsound modifies the path when running on macOS so that it has access to that built-in module...
I think that'll be the best possible solution... I'll try figuring it out during the long holiday weekend...

This is now fixed on master. I have not added PyObjC as a hard dependency, but I print out a suggestion that you aught to install it. Even without it, the code still works on Python 3 on Mac, by shelling out to Mac OS's built-in python.


*: tests with client TLS fail

=== RUN   TestCtlV3DelClientTLS
--- FAIL: TestCtlV3DelClientTLS (0.22s)
	ctl_v3_kv_test.go:200: delTest #0-0: ctlV3Put error (read /dev/ptmx: input/output error (expected "OK", got ["Error:  rpc error: code = 13 desc = transport is closing\r\n"]))

Fails with current master?

Think it's cert issue. Tried regenerating cfssl fixtures but still fails...

I can repro this on OSX because it's picky about certs tracing back to the root CA but it seems to work OK on my gentoo install. Not seeing the failures on semaphore. Are these failures on Ubuntu?

Yeah on my Ubuntu machine. Rebuilding master branch on semaphore and it seems works fine :0

It's failing when we test Semaphore with Go 1.7. Set it back to Go 1.6.3 for now.

I'm seeing a failure of grpc to connect properly on v3.1.0-alpha.0 and v3.0.6 - error is different, but symptom is the same.  Compiled with go 1.7 clients aren't able to connected, compiled with go 1.6.2 clients can connect.
v3.1.0-alpha.0
$ etcd --listen-peer-urls=https://<IP_ADDRESS>:7001 --listen-client-urls=https://<IP_ADDRESS>:4001  --advertise-client-urls=https://<IP_ADDRESS>:4001 --cert-file openshift.local.config/master/etcd.server.crt --key-file openshift.local.config/master/etcd.server.key --peer-cert-file openshift.local.config/master/etcd.server.crt  --peer-key-file openshift.local.config/master/etcd.server.key --initial-advertise-peer-urls https://<IP_ADDRESS>:7001 --initial-cluster=default=https://<IP_ADDRESS>:7001
2016-09-17 23:52:27.321334 I | etcdmain: etcd Version: 3.1.0-alpha.0
2016-09-17 23:52:27.321420 I | etcdmain: Git SHA: 5c20531
2016-09-17 23:52:27.321426 I | etcdmain: Go Version: go1.7
2016-09-17 23:52:27.321428 I | etcdmain: Go OS/Arch: darwin/amd64
2016-09-17 23:52:27.321431 I | etcdmain: setting maximum number of CPUs to 8, total number of available CPUs is 8
2016-09-17 23:52:27.321439 W | etcdmain: no data-dir provided, using default data-dir ./default.etcd
2016-09-17 23:52:27.321462 I | embed: peerTLS: cert = openshift.local.config/master/etcd.server.crt, key = openshift.local.config/master/etcd.server.key, ca = , trusted-ca = , client-cert-auth = false
2016-09-17 23:52:27.322034 I | embed: listening for peers on https://<IP_ADDRESS>:7001
2016-09-17 23:52:27.322071 I | embed: listening for client requests on <IP_ADDRESS>:4001
2016-09-17 23:52:27.323685 I | etcdserver: name = default
2016-09-17 23:52:27.323695 I | etcdserver: data dir = default.etcd
2016-09-17 23:52:27.323699 I | etcdserver: member dir = default.etcd/member
2016-09-17 23:52:27.323702 I | etcdserver: heartbeat = 100ms
2016-09-17 23:52:27.323705 I | etcdserver: election = 1000ms
2016-09-17 23:52:27.323708 I | etcdserver: snapshot count = 10000
2016-09-17 23:52:27.323716 I | etcdserver: advertise client URLs = https://<IP_ADDRESS>:4001
2016-09-17 23:52:27.323719 I | etcdserver: initial advertise peer URLs = https://<IP_ADDRESS>:7001
2016-09-17 23:52:27.323725 I | etcdserver: initial cluster = default=https://<IP_ADDRESS>:7001
2016-09-17 23:52:27.428496 I | etcdserver: starting member 3092679e8c56a1a5 in cluster e989df3141e943e1
2016-09-17 23:52:27.428542 I | raft: 3092679e8c56a1a5 became follower at term 0
2016-09-17 23:52:27.428560 I | raft: newRaft 3092679e8c56a1a5 [peers: [], term: 0, commit: 0, applied: 0, lastindex: 0, lastterm: 0]
2016-09-17 23:52:27.428570 I | raft: 3092679e8c56a1a5 became follower at term 1
2016-09-17 23:52:27.431765 I | etcdserver: starting server... [version: 3.1.0-alpha.0, cluster version: to_be_decided]
2016-09-17 23:52:27.431794 I | embed: ClientTLS: cert = openshift.local.config/master/etcd.server.crt, key = openshift.local.config/master/etcd.server.key, ca = , trusted-ca = , client-cert-auth = false
2016-09-17 23:52:27.431934 E | etcdserver: cannot monitor file descriptor usage (cannot get FDUsage on darwin)
2016-09-17 23:52:27.432247 I | membership: added member 3092679e8c56a1a5 [https://<IP_ADDRESS>:7001] to cluster e989df3141e943e1
2016-09-17 23:52:28.230258 I | raft: 3092679e8c56a1a5 is starting a new election at term 1
2016-09-17 23:52:28.230316 I | raft: 3092679e8c56a1a5 became candidate at term 2
2016-09-17 23:52:28.230321 I | raft: 3092679e8c56a1a5 received vote from 3092679e8c56a1a5 at term 2
2016-09-17 23:52:28.230330 I | raft: 3092679e8c56a1a5 became leader at term 2
2016-09-17 23:52:28.230335 I | raft: raft.node: 3092679e8c56a1a5 elected leader 3092679e8c56a1a5 at term 2
2016-09-17 23:52:28.230458 I | etcdserver: setting up the initial cluster version to 3.1
2016-09-17 23:52:28.230518 I | embed: ready to serve client requests
2016-09-17 23:52:28.230545 I | etcdserver: published {Name:default ClientURLs:[https://<IP_ADDRESS>:4001]} to cluster e989df3141e943e1
2016-09-17 23:52:28.230836 I | embed: serving client requests on [::]:4001
2016-09-17 23:52:28.234083 N | membership: set the initial cluster version to 3.1
2016-09-17 23:52:28.234140 I | api: enabled capabilities for version 3.1
2016/09/17 23:52:28 transport: http2Client.notifyError got notified that the client transport was broken unexpected EOF.
...
// hundreds of identical lines
...
2016/09/17 23:52:28 transport: http2Client.notifyError got notified that the client transport was broken unexpected EOF.
2016/09/17 23:52:28 grpc: addrConn.resetTransport failed to create client transport: connection error: desc = "transport: write tcp [::1]:57914->[::1]:4001: write: broken pipe"; Reconnecting to {"[::]:4001" <nil>}

It's definitely 1.7 vs 1.6.2 for me - 3.0.6, 3.0.9, and 3.1.0-alpha.0 exhibited same behavior.

Also does not work in Go 1.7.1 either.

@gyuho
In go 1.7 release note
The server's Serve method now only enables HTTP/2 support if the Server.TLSConfig field is nil
or includes "h2" in its TLSConfig.NextProtos.

This breaks us. We need to explicitly set h1 in the nextProtos.

@xiang90 Ok will try that. Thanks.


Add incanter Latest

Incanter is a Clojure-based, R-like platform for statistical computing and graphics.

Thank you for the contribution. It was merged directly as 3564a098a22e55c1dd7aa1605b901449903a5344 to keep commit history cleaner. Your contribution is still credited to you.


Add rayon parallel sort example

I initially went with an example like this:
extern crate rand;
extern crate rayon;

use rand::Rng;
use rayon::prelude::*;

fn main() {
    let mut vec = vec![0i64; 1_000_000];

    // Generating random numbers to be sorted later
    vec.par_iter_mut().for_each(|p| *p = rand::weak_rng().gen::<i64>());

    // Using Rayon's Parallel QuickSort to sort the vector
    vec.par_sort_unstable();
}

but that seemed overly simplistic(I think you are more likely to sort strings than just integers).
Had no idea what to write in the description, under the code...

Pushed the fixes with some of the points unaddressed(rayon = "*" & links), because I want to see if I manage to convince you otherwise.

I want to see if I manage to convince you otherwise.

No need for convincing. I'm already a fan of tutorial teaching style resources. And one of the ideas is to eventually expand cookbook to "Rust By Example" style or even merge with it. But we are currently brainstorming for ideas

Sorry, I'm low on time right now.
I'll follow-up on this tommorow

No rush :)

Hi @lilianmoraru. I'm back from holidays so I should be to have a more meaningful discussion in regard to this example. Are you willing to continue your work on it?

I was also on vacation.
Let's continue with this.
A few things to note here:

I rebased on the current master
I moved the other explanations under concurrency, to be on top of the code(more consistent with the recent changes in the other examples).
I removed the explanations from inside the code as requested, but the explanation was so disconnected from the code that I myself would not understand what it was talking about(why I've decided to put the explanations there in the first place)... So, I've marked the areas(again, another style borrowed from Qt, this time from Qt examples), to show the reader about which areas of the code the explanation is talking about.
I still think the explanations should be right beside the code, so the user is connected better to what the explanation talks about - but I won't insist, as long as this helps to close this PR(be it merged or not).

I was planning to remove the multiple options and stable sorting links(which I think are important to the reader, ex: comes to see how to sort, but wants a stable sorting algorithm, lucky him, he had a link to the other sorting algorithms and didn't think there are no options for him), but I saw the Julia Set link and left them in(again, because I think that we should leave more info/options to the reader).

Merged :smiley:  Thanks @lilianmoraru!
Thanks for the cleanup It was on my todo list for way to long :)
I'm not sure about the numbered pseudo-links as we lack proper tooling to turn them into a real hyperlink or a split code/tutorial page like in qt examples case, making it look a little awkward and nonconformant with other examples style. But we are soliciting ideas for design and form overhaul so I'm willing to bend the rules here to get some input from users :)


Static parts in templates

Hi,
this pull request is trying to solve the use case, where we might have static components in a template. e.g.:
var template = {
  key : 'last',
  value: {
    param: '$.param',
    fields: '$.fields'
  }
};
var data = { param: 56, fields: {}};

// transform now returns:
{ key: undefined, value: { param: 56, fields: {} } }

// instead of the more correct:
{ key: 'last', value: { param: 56, fields: {} } }

I'm not a 100% certain that all JSONPaths must start with a $, but I haven't found an example on the internet that doesn't. We can always expand it to a regexp that checks for all the JSONPath operators.
The second commit, just removes the node_modules folder from the repo and ignores it.
Let me know what you think.
Cheers,
Chris

I'm not sure about paths always starting with a dollar sign. I will look into this today.

Any updates on this one?

This solution is fine, though it may be smarter to require paths be explicitely denoted:
import {Path} from 'jsonpath-object-transform'

const template = {
  key: 'last',
  value: {
    param: Path('$.param'),
    fields: Path('$.fields')
  }
}

Might implement that later… but for now checking for $ is fine.


Unable to install Cilium on GKE 1.24

Is there an existing issue for this?

[X] I have searched the existing issues

What happened?
I'm unable to install cilium and it seems the flags provided by Helm break the nodes.
helm install cilium cilium/cilium --version 1.12.1 \
  --namespace kube-system \
  --set nodeinit.enabled=true \
  --set nodeinit.reconfigureKubelet=true \
  --set nodeinit.removeCbrBridge=true \
  --set cni.binPath=/home/kubernetes/bin \
  --set gke.enabled=true \
  --set ipam.mode=kubernetes \
  --set hubble.relay.enabled=true \
  --set hubble.ui.enabled=true \
  --set ipv4NativeRoutingCIDR=<IP_ADDRESS>/18

 REDACTED  MCW0CDP3YY  tmp  $  kg nodes
W0901 13:13:32.602478   50028 gcp.go:120] WARNING: the gcp auth plugin is deprecated in v1.22+, unavailable in v1.25+; use gcloud instead.
To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke
NAME                         STATUS     ROLES    AGE     VERSION
gke-dev-main-0806e16a-06d7   NotReady   <none>   8m46s   v1.24.2-gke.1900
gke-dev-main-0806e16a-4sjg   NotReady   <none>   8m44s   v1.24.2-gke.1900
gke-dev-main-19ccc9b4-p0b7   NotReady   <none>   8m47s   v1.24.2-gke.1900
gke-dev-main-19ccc9b4-z116   NotReady   <none>   8m49s   v1.24.2-gke.1900

 REDACTED  MCW0CDP3YY  tmp  $  kg po -o wide -n kube-system | grep cilium
W0901 13:15:01.806292   50065 gcp.go:120] WARNING: the gcp auth plugin is deprecated in v1.22+, unavailable in v1.25+; use gcloud instead.
To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke
cilium-4s99g                             0/1     Init:0/5            0          4m17s   <IP_ADDRESS>   gke-dev-main-19ccc9b4-p0b7   <none>           <none>
cilium-8c5c5                             0/1     Init:0/5            0          4m16s   <IP_ADDRESS>   gke-dev-main-0806e16a-06d7   <none>           <none>
cilium-fk4zg                             0/1     Init:0/5            0          4m17s   <IP_ADDRESS>   gke-dev-main-19ccc9b4-z116   <none>           <none>
cilium-node-init-6xnnl                   0/1     ContainerCreating   0          4m18s   <IP_ADDRESS>   gke-dev-main-19ccc9b4-z116   <none>           <none>
cilium-node-init-lqbn9                   0/1     ContainerCreating   0          4m18s   <IP_ADDRESS>   gke-dev-main-0806e16a-4sjg   <none>           <none>
cilium-node-init-vdnzj                   0/1     ContainerCreating   0          4m18s   <IP_ADDRESS>   gke-dev-main-19ccc9b4-p0b7   <none>           <none>
cilium-node-init-zs2j4                   0/1     ContainerCreating   0          4m18s   <IP_ADDRESS>   gke-dev-main-0806e16a-06d7   <none>           <none>
cilium-operator-6f5c6f768d-zqwf6         1/1     Running             0          4m18s   <IP_ADDRESS>   gke-dev-main-0806e16a-06d7   <none>           <none>
cilium-t8mz6                             0/1     Init:1/5            0          4m18s   <IP_ADDRESS>   gke-dev-main-0806e16a-4sjg   <none>           <none>

 REDACTED  MCW0CDP3YY  tmp  $  kg node gke-dev-main-19ccc9b4-z116 -o yaml | yq '.status.conditions | .[] | select(.status == "True" or .status == "Unknown")'
W0901 13:21:36.942178   50349 gcp.go:120] WARNING: the gcp auth plugin is deprecated in v1.22+, unavailable in v1.25+; use gcloud instead.
To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke
lastHeartbeatTime: "2022-09-01T12:19:54Z"
lastTransitionTime: "2022-09-01T12:14:52Z"
message: Found 23 matching logs, which meets the threshold of 5
reason: FrequentContainerdRestart
status: "True"
type: FrequentContainerdRestart
lastHeartbeatTime: "2022-09-01T12:19:54Z"
lastTransitionTime: "2022-09-01T12:14:50Z"
message: Found 23 matching logs, which meets the threshold of 5
reason: FrequentKubeletRestart
status: "True"
type: FrequentKubeletRestart
lastHeartbeatTime: "2022-09-01T12:10:20Z"
lastTransitionTime: "2022-09-01T12:11:23Z"
message: Kubelet stopped posting node status.
reason: NodeStatusUnknown
status: Unknown
type: MemoryPressure
lastHeartbeatTime: "2022-09-01T12:10:20Z"
lastTransitionTime: "2022-09-01T12:11:23Z"
message: Kubelet stopped posting node status.
reason: NodeStatusUnknown
status: Unknown
type: DiskPressure
lastHeartbeatTime: "2022-09-01T12:10:20Z"
lastTransitionTime: "2022-09-01T12:11:23Z"
message: Kubelet stopped posting node status.
reason: NodeStatusUnknown
status: Unknown
type: PIDPressure
lastHeartbeatTime: "2022-09-01T12:10:20Z"
lastTransitionTime: "2022-09-01T12:11:23Z"
message: Kubelet stopped posting node status.
reason: NodeStatusUnknown
status: Unknown
type: Ready

 REDACTED  MCW0CDP3YY  tmp  $  cilium status
level=warning msg="WARNING: the gcp auth plugin is deprecated in v1.22+, unavailable in v1.25+; use gcloud instead." subsys=klog
level=warning msg="To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke" subsys=klog
    /¯¯\
 /¯¯\__/¯¯\    Cilium:         1 errors, 4 warnings
 \__/¯¯\__/    Operator:       1 errors
 /¯¯\__/¯¯\    Hubble:         disabled
 \__/¯¯\__/    ClusterMesh:    disabled
    \__/

Deployment        cilium-operator    Desired: 1, Unavailable: 1/1
DaemonSet         cilium             Desired: 4, Unavailable: 4/4
Containers:       cilium             Pending: 4
                  cilium-operator    Running: 1
Cluster Pods:     0/5 managed by Cilium
Image versions    cilium             quay.io/cilium/cilium:v1.12.1@sha256:ea2db1ee21b88127b5c18a96ad155c25485d0815a667ef77c2b7c7f31cab601b: 4
                  cilium-operator    quay.io/cilium/operator-generic:v1.12.1@sha256:93d5aaeda37d59e6c4325ff05030d7b48fabde6576478e3fdbfb9bb4a68ec4a1: 1
Errors:           cilium-operator    cilium-operator    1 pods of Deployment cilium-operator are not ready
                  cilium             cilium             4 pods of DaemonSet cilium are not ready
Warnings:         cilium             cilium-t8mz6       pod is pending
                  cilium             cilium-4s99g       pod is pending
                  cilium             cilium-8c5c5       pod is pending
                  cilium             cilium-fk4zg       pod is pending

Cilium Version
REDACTED  MCW0CDP3YY  tmp  $  cilium version
level=warning msg="WARNING: the gcp auth plugin is deprecated in v1.22+, unavailable in v1.25+; use gcloud instead." subsys=klog
level=warning msg="To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke" subsys=klog
cilium-cli: v0.12.2 compiled with go1.19 on darwin/amd64
cilium image (default): v1.12.1
cilium image (stable): v1.12.1
cilium image (running): v1.12.1

Kernel Version
Managed Nodes
Kubernetes Version
 REDACTED  MCW0CDP3YY  tmp  $  k version
W0901 13:12:19.990707   49964 gcp.go:120] WARNING: the gcp auth plugin is deprecated in v1.22+, unavailable in v1.25+; use gcloud instead.
To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.1", GitCommit:"3ddd0f45aa91e2f30c70734b175631bec5b5825a", GitTreeState:"clean", BuildDate:"2022-05-24T12:17:11Z", GoVersion:"go1.18.2", Compiler:"gc", Platform:"darwin/amd64"}
Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.2-gke.1900", GitCommit:"d95ef8792724f6b489a64fe5c7d8f1fb3f9422bb", GitTreeState:"clean", BuildDate:"2022-07-15T09:24:44Z", GoVersion:"go1.18.3b7", Compiler:"gc", Platform:"linux/amd64"}

Sysdump
No response
Relevant log output
No response
Anything else?
No response
Code of Conduct

[X] I agree to follow this project's Code of Conduct

Cilium deploys nicely on GKE 1.23
 REDACTED  MCW0CDP3YY  tmp  $  cilium status
level=warning msg="WARNING: the gcp auth plugin is deprecated in v1.22+, unavailable in v1.25+; use gcloud instead." subsys=klog
level=warning msg="To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke" subsys=klog
    /¯¯\
 /¯¯\__/¯¯\    Cilium:         OK
 \__/¯¯\__/    Operator:       OK
 /¯¯\__/¯¯\    Hubble:         OK
 \__/¯¯\__/    ClusterMesh:    disabled
    \__/

DaemonSet         cilium             Desired: 4, Ready: 4/4, Available: 4/4
Deployment        hubble-relay       Desired: 1, Ready: 1/1, Available: 1/1
Deployment        hubble-ui          Desired: 1, Ready: 1/1, Available: 1/1
Deployment        cilium-operator    Desired: 2, Ready: 2/2, Available: 2/2
Containers:       cilium             Running: 4
                  hubble-relay       Running: 1
                  hubble-ui          Running: 1
                  cilium-operator    Running: 2
Cluster Pods:     8/8 managed by Cilium
Image versions    cilium             quay.io/cilium/cilium:v1.12.1@sha256:ea2db1ee21b88127b5c18a96ad155c25485d0815a667ef77c2b7c7f31cab601b: 4
                  hubble-relay       quay.io/cilium/hubble-relay:v1.12.1@sha256:646582b22bf41ad29dd7739b12aae77455ee5757b9ee087f2d45d684afef5fa1: 1
                  hubble-ui          quay.io/cilium/hubble-ui:v0.9.1@sha256:baff611b975cb12307a163c0e547e648da211384eabdafd327707ff2ec31cc24: 1
                  hubble-ui          quay.io/cilium/hubble-ui-backend:v0.9.1@sha256:c4b86e0d7a38d52c6ea3d9d7b17809e5212efd97494e8bd37c8466ddd68d42d0: 1
                  cilium-operator    quay.io/cilium/operator-generic:v1.12.1@sha256:93d5aaeda37d59e6c4325ff05030d7b48fabde6576478e3fdbfb9bb4a68ec4a1: 2
 REDACTED  MCW0CDP3YY  tmp  $  k version
W0901 13:53:07.278849   52247 gcp.go:120] WARNING: the gcp auth plugin is deprecated in v1.22+, unavailable in v1.25+; use gcloud instead.
To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.1", GitCommit:"3ddd0f45aa91e2f30c70734b175631bec5b5825a", GitTreeState:"clean", BuildDate:"2022-05-24T12:17:11Z", GoVersion:"go1.18.2", Compiler:"gc", Platform:"darwin/amd64"}
Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.7-gke.1400", GitCommit:"3cdaae9a00a0ebd5b6fe15279d5da23ced7d85ba", GitTreeState:"clean", BuildDate:"2022-06-14T09:26:54Z", GoVersion:"go1.17.10b7", Compiler:"gc", Platform:"linux/amd64"}

Duplicate of https://github.com/cilium/cilium/issues/21049

Thanks, it wasn't clear how 21049 is linked to this issue.

On a sidenote, I need to complain to Google about their log severities.


[Backport] add sequenceName and currentCheckPoint for backwards compatibility (#8864)

Backport of #8864 to 0.16.1-incubating

Closing, this is not actually applicable to 0.16.0, the source of the rolling upgrade issue was https://github.com/apache/incubator-druid/pull/8707, which is not in 0.16.0


Update indices of functions in name section while injecting gas function

Desc
Here we forgetting to update the indices in the name section

I am curious, why do you need this?
I am asking because this part of code is certainly sensitive since it's used in on-chain logic, and on the other hand, we strive to avoid dealing with the names section since it's tricky (and perhaps even not sound in the current impl).

I am curious, why do you need this?
I am asking because this part of code is certainly sensitive since it's used in on-chain logic, and on the other hand, we strive to avoid dealing with the names section since it's tricky (and perhaps even not sound in the current impl).

Ref to  https://github.com/paritytech/cargo-contract/pull/131, https://github.com/paritytech/parity-wasm/pull/300 and https://github.com/paritytech/wasmi/compare/master...patractlabs:v0.6.2

I am curious, why do you need this?
I am asking because this part of code is certainly sensitive since it's used in on-chain logic, and on the other hand, we strive to avoid dealing with the names section since it's tricky (and perhaps even not sound in the current impl).

We implemented wasm backtrace for contracts in @patractlabs/europa, for example, panic in the deploy function of flipper, the result is like:
Trap(
    Trap {
        kind: Unreachable,
        wasm_trace: [
            "deploy[17]",
            "<unknown>[27]",
        ],
    },
)

And there will be more backtraces if we enable the debug build.

Ref to  https://github.com/paritytech/cargo-contract/pull/131, https://github.com/paritytech/parity-wasm/pull/300 and https://github.com/paritytech/wasmi/compare/master...patractlabs:v0.6.2

I am curious, why do you need this?
I am asking because this part of code is certainly sensitive since it's used in on-chain logic, and on the other hand, we strive to avoid dealing with the names section since it's tricky (and perhaps even not sound in the current impl).

We implemented wasm backtrace for contracts in @patractlabs/europa, for example, panic in the deploy function of flipper, the result is like:
Trap(
    Trap {
        kind: Unreachable,
        wasm_trace: [
            "deploy[17]",
            "<unknown>[27]",
        ],
    },
)

And there will be more backtraces if we enable the debug build.

Just to err on the safe side, do you think it's possible to demarcate the execution for debugging and on-chain use-cases? FWIW, I am not so keen to see a complete reallocation and rebuilding of the name section during (already heavy-ish) contract preparation step
Also, @athei what do you think about this? Does this concern Seal? Do name sections ever pass thru the preparation/put code stage, and does gas injection observe it?

Just to err on the safe side, do you think it's possible to demarcate the execution for debugging and on-chain use-cases? FWIW, I am not so keen to see a complete reallocation and rebuilding of the name section during (already heavy-ish) contract preparation step
Also, @athei what do you think about this? Does this concern Seal? Do name sections ever pass thru the preparation/put code stage, and does gas injection observe it?

Also, @athei what do you think about this? Does this concern Seal? Do name sections ever pass thru the preparation/put code stage, and does gas injection observe it?

There is no exhaustive matching on the sections. Any unknown section will just pass through to gas injection and hence will be affected by this code. Maybe we should change that to deny deployment if any unknown section (like name section) is encountered.

Also, @athei what do you think about this? Does this concern Seal? Do name sections ever pass thru the preparation/put code stage, and does gas injection observe it?

There is no exhaustive matching on the sections. Any unknown section will just pass through to gas injection and hence will be affected by this code. Maybe we should change that to deny deployment if any unknown section (like name section) is encountered.

There is no exhaustive matching on the sections. Any unknown section will just pass through to gas injection and hence will be affected by this code. Maybe we should change that to deny deployment if any unknown section (like name section) is encountered.

It would be nice to add a field in the runtime Config for enabling or not for the name section(debug contracts), people write contracts, why we deny them debug their contracts using the name sections?

There is no exhaustive matching on the sections. Any unknown section will just pass through to gas injection and hence will be affected by this code. Maybe we should change that to deny deployment if any unknown section (like name section) is encountered.

It would be nice to add a field in the runtime Config for enabling or not for the name section(debug contracts), people write contracts, why we deny them debug their contracts using the name sections?

It is just a security precaution to not have sections in wasm-blobs on-chain which we do not make use of. So why should we allow deploying this section when it is currently unused?

It is just a security precaution to not have sections in wasm-blobs on-chain which we do not make use of. So why should we allow deploying this section when it is currently unused?

I would be fine with merging if you add a process_name_section: ProcessNames parameter to inject_gas_counter with a big fat warning that this should never ever set to true whenever untrusted code is processed:
enum ProcessNames {
    No,
    UnsafeYes,
}

I would be fine with merging if you add a process_name_section: ProcessNames parameter to inject_gas_counter with a big fat warning that this should never ever set to true whenever untrusted code is processed:
enum ProcessNames {
    No,
    UnsafeYes,
}

@athei @pepyakin
Hi, we(PatractLabs) is developing europa which is a tool to debug pallet-contracts. So when run a contract and panic in WASM, in europa the WASM trace could be printed to show where the code meet the panic. And in future, we would try to add WASM debug in Europa env, and could use gdb or lldb to debug contract in a chain runtime env. Therefore we need name section to bring in the function symbol for source file. And europa v0.2 report for proposal will be summitted next week, guys could see what we do, it's a nice tool to location the bug in code.
though ink! provides a whole offchain env to run test case to debug code, but this just a could debug in one contract. Like current defi in ETH, the real useful thing is the contract could call another contract and combine together, so that europa is a good tool to debug crossing different contracts.
And on the other hand, wasm-utils is a common lib to provide some useful tools to modify WASM, I think in this level, we should not assume the below level (like pallet-contracts, or someone create another contract runtime module) whether needs the name section. Whether should deny name section is depends on the below level, for example, pallet-contracts should check whether needs name section, and in our forked version pallet-contracts, we could ignore this check.
And in another way, I think this feature could be provided by conditional compilation, like we could design this as a features #[name_section_support] or some way.

@athei @pepyakin
Hi, we(PatractLabs) is developing europa which is a tool to debug pallet-contracts. So when run a contract and panic in WASM, in europa the WASM trace could be printed to show where the code meet the panic. And in future, we would try to add WASM debug in Europa env, and could use gdb or lldb to debug contract in a chain runtime env. Therefore we need name section to bring in the function symbol for source file. And europa v0.2 report for proposal will be summitted next week, guys could see what we do, it's a nice tool to location the bug in code.
though ink! provides a whole offchain env to run test case to debug code, but this just a could debug in one contract. Like current defi in ETH, the real useful thing is the contract could call another contract and combine together, so that europa is a good tool to debug crossing different contracts.
And on the other hand, wasm-utils is a common lib to provide some useful tools to modify WASM, I think in this level, we should not assume the below level (like pallet-contracts, or someone create another contract runtime module) whether needs the name section. Whether should deny name section is depends on the below level, for example, pallet-contracts should check whether needs name section, and in our forked version pallet-contracts, we could ignore this check.
And in another way, I think this feature could be provided by conditional compilation, like we could design this as a features #[name_section_support] or some way.

Closed for lack of activity.


The Ugly Generic,might as well not have it.

Go doesn't really need a generic, and it's better to optimize performance than to get a generic for Go to have a generic as a feature.
Some developers with experience in other languages want to use features in Go that they use in other languages, and then they ask Go to add this or that feature (there's always someone who wants to add try-catch to Go's error handling).
For now, many people don't feel that Go needs generics very much, and the existing generic proposal isn't very well liked (many people complain that it doesn't use <> but instead uses []).
Go is Go, Go has its own style, simple, efficient, and engineered.

This is an issue tracker. I'm closing this.


2020-10-19 post

[x] I confirm that this blog post was made under duress

Oh come on it wasn't that bad though was it?!
Thanks!!


Add  bias to gptj

What does this PR do?
Transformer4.45 has the Dynamic cache updates(https://github.com/huggingface/transformers/pull/31421) removing
self.bias causing the failure. Until we investigate further  and update the code based on the new transformer, we are putting bias back to GaudiGPTJAttention.init()

Fixes # (issue)
GAUDI2_CI=1  RUN_SLOW=1 python3.10 -m pytest tests/test_text_generation_example.py -k token0-EleutherAI/gpt-j-6b-1
File "/usr/local/lib/python3.10/dist-packages/optimum/habana/transformers/models/gptj/modeling_gptj.py", line 124, in _attn
causal_mask = self.bias[:, :, key_length - query_length : key_length, :key_length]
AttributeError: 'GaudiGPTJAttention' object has no attribute 'bias'

@atakaha  Could you check on this PR? Transformer4.45 has the Dynamic cache updates (https://github.com/huggingface/transformers/pull/31421) which removed self.bias, and that caused an error on our side. I tried to update the model file based on their new logic but found that update_sincos_cache() also using self.bias as well. For now I put the self.bias back to our model file so the test goes through. Could you please review the transformer PR31421 and then see if you can update the gptj model file accordingly?

looks good to me.
I'll study how to apply the PR to GPTJ.


@joeyAghion => Fix CSS regression

I actually introduced this! And not the primer thing...

(cc @briansw fixes the misplaced button)

Thanks for fixing and linking!


MF-1198 - Add errors package tests

Add unit tests to errors package from pkg/.
Resolves #1198

Did you consider to include test cases when error is nil (for any of these tests)?

Did you consider to include test cases when error is nil (for any of these tests)?
Actually, a very good idea :D and an omission from my side...

Codecov Report

Merging #1207 into master will decrease coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1207      +/-   ##
==========================================
- Coverage   77.06%   76.98%   -0.09%
==========================================
  Files         103      104       +1
  Lines        6893     6934      +41
==========================================
+ Hits         5312     5338      +26
- Misses       1197     1208      +11
- Partials      384      388       +4

Impacted Files
Coverage Δ

pkg/errors/errors.go
63.41% <0.00%> (ø)

Continue to review full report at Codecov.

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


TBD[Головна сторінка]  The scroll page of 'Оберіть напрям гуртків' doesn't start with the last card from a previous scroll page

Environment: Windows, Google Chrome 88.0.4324.190 (64-bit)
Reproducible: always.
Build found: https://speak-ukrainian.org.ua/dev/
Steps to reproduce

Go to https://speak-ukrainian.org.ua/dev/
Scroll down to 'Оберіть напрям гуртків' cards
Pay attention to the last card on the scroll page
Swipe to the next scroll page
Pay attention to the first card on this scroll page

Actual result
This scroll page doesn't contain the last card from a previous scroll page

Expected result
The scroll page should start with the last card from a previous scroll page

User story and test case links
E.g.: "User story #166, #108
Labels to be added
"Bug", Priority ("pri: low"), Severity ("severity: minor"), Type ("UI").

it`s not bag


json-serde zig: streaming md5

(for  canada 100)
Before:
User time (seconds): 3.61
Maximum resident set size (kbytes): 542020

After:
User time (seconds): 3.45
Maximum resident set size (kbytes): 114816

Thanks for the PR! It's a great optimization, but I think it makes the zig solution incomparable to others, I might need more time to think about it.

How about 2-streaming.zig ? I also added streaming md5 for go and rust. (go doesn't have any performance improvements, it buffers the entire json before writing)


Initial release of rqlite Helm chart

Still in development, but fairly comprehensive already.  Submitting a draft PR to begin the review process.
Remaining work items:

readonly nodes
cleaner support for automated backups (it's possible now through extraFiles and extraArgs but I want to investigate higher level abstractions)

While going through active development, I decided to include the develop branch in the chart release action so that 0.x versions will get released, so that aspect can also be tested before moving to 1.0.0.
There's a lot here, especially in values.yaml, so I'd be grateful for any corrections or suggestions on best practices.
The template files themselves are probably less important to review, but feel free.  Helm (Go+sprig) template syntax can be a bit arcane (and not in the good way).

@otoolep  I also wanted to ask, is there a reason not to use -raft-cluster-remove-shutdown=true by default?

@otoolep I also wanted to ask, is there a reason not to use -raft-cluster-remove-shutdown=true by default?

There isn't a strong correlation between shutting down a node, and wanting to actually remove it from the cluster IMHO. Removing a node from a cluster changes the quorum requirements, and that's not something that should happen by default. It should only happen if someone explicitly requests this mode. I believe this is the way Consul works.

There isn't a strong correlation between shutting down a node, and wanting to actually remove it from the cluster IMHO.

 I neglected to mention an important detail:
I was thinking about this specifically in the context of read-only nodes.  I was contemplating how Horizontal Pod Autoscaling (HPA) would work with the readonly pool, once I implement it.  Because supporting autoscaling for readonly replicas would be a nice capability.
HPA works basically equivalently to kubectl scale statefulset/rqlite --replicas=<n>.  When scaling up, readonly nodes will join the cluster just fine.  When scaling down, per https://rqlite.io/docs/clustering/read-only-nodes/#handling-failure it will generate quite a lot of noise in the remaining nodes' logs (though AFAICT be otherwise harmless?).
-raft-cluster-remove-shutdown=true would handle this for scale-down operations.  Of course, it does mean, as you say, the readonly node will leave the cluster even on standard restarts.  But is this a problem?

Although, in the context of voters, it does occur to me the chart should offer some means of implementing https://rqlite.io/docs/clustering/general-guidelines/#dealing-with-failure
More generally, if the user explicitly lowers the replicaCount chart value, it's probably not unreasonable to assume they do want to jettison those nodes. I could generate peers.json -- AFAICT everything needed to generate that file is known at chart deployment time (using FQDNs instead of IPs for address of course) -- but I think this could be problematic in the context of readonly replicas and autoscaling them.
Once I get the readonly stuff implemented I'll experiment with that.
Alternatively, as suggested by the docs, I relegate peers.json to "break glass in case of emergency" situations and the chart provides some other logic to support scaling voters down.  This is trickier than it sounds, because it means inside the pod there needs to be a way for it to dynamically determine if it should leave the cluster on termination.
It's tricky but not impossible: I could have a custom preStop script that uses curl to call the K8s API to fetch the current number of replicas for the StatefulSet it belongs to, and if the current number is less than its node id, gracefully leave the cluster, otherwise don't.  (Ok, I see curl isn't in the rqlite container, so maybe a sidecar that uses docker.io/curlimages/curl.) That's clever, but I'll need to chew on this for a bit to decide if it's too clever.

Last commit adds support for read-only nodes.

-raft-cluster-remove-shutdown=true would handle this for scale-down operations. Of course, it does mean, as you say, the readonly node will leave the cluster even on standard restarts. But is this a problem?

I included -raft-cluster-remove-shutdown=true for the readonly pods as I couldn't see any obvious downside.  With cursory testing, dynamic scaling up and down has behaved flawlessly. Although I'd like to do a bit more testing with heavy mutation load during autoscale events to ensure readonly nodes rejoining the cluster don't serve up stale data from their previous membership.
That is to say, verify the readiness probe isn't reporting ready before the node completes syncing data from the leader.  (I did check this hastily and it seemed ok but I wasn't driving any real load.) I know there's the freshness parameter but as I understand it that's more a knob to tune the window of eventual consistency in the face of network blips than it is to deal with a node rejoining a cluster after a day (or whatever).
The only thing left on my TODO list is for the chart to provide a mechanism for generating peers.json as a way to recover from permanent node loss, per the procedure documented at https://rqlite.io/docs/clustering/general-guidelines/#dealing-with-failure

The only thing left on my TODO list is for the chart to provide a mechanism for generating peers.json as a way to recover from permanent node loss

This is now down, via the useStaticPeers chart option.
I think this is all fairly baked at this point, so I'm removing draft status from the PR.

Actually retagging as draft: it occurs to me that the procedure I documented for shrinking the number of voters has problems.
The procedure was to first use the API to remove the node, then stop the pods.  The problem with this is it will take up to 20 seconds for the removed node(s) to fail their readiness probe, during which time they will still receive requests via the K8s Service, and return 503s.
For that to work, we'd need some way to first mark the nodes as soft-down, such that they fail the readness probe to get removed from the K8s Service (and therefore stop taking new traffic), but continue to service in-flight transactions.  Then, once removed from the K8s Service, they can be removed from the cluster.
So instead I guess the right order is to first shutdown the nodes, then remove them.  The main problems with this are:

If you want to shutdown more than N/2-1 nodes, you need to do them in batches of at must N/2-1 nodes or else the decom process will cause loss of quorum.
The procedure will generate a lot of noise in the logs (nodes being shutdown before being removed)
I'm passing the value of  replicaCount to -bootstrap-expect which means when the replicaCount changes, all nodes in the cluster roll.  This is really suboptimal for a decom procedure where you really don't want the cluster churning six ways from Sunday.

I think 1 and 2 are tolerable.  3 requires a change to how I'm passing -bootstrap-expect.  I can build on the trick I used for useStaticPeers (in which I use a ConfigMap value to trigger the behavior rather than changing the pod spec, where the latter case will trigger a rollout restart whereas the former won't).

Thanks for this. When it merges, I'll add some notes about it to rqlite.io.

Thanks for this. When it merges, I'll add some notes about it to rqlite.io.

Sounds great. I'll merge it later today or tomorrow after a bit more testing on different cloud providers.

Verified basic deployment on EKS, LKE, and microk8s clusters.  Looks good.

Although I'd like to do a bit more testing with heavy mutation load during scale events to ensure readonly nodes rejoining the cluster don't serve up stale data from their previous membership.

This worked well. Ran a test where I:

scaled the readonly pool up to 7 replicas
set a numeric column in a table to 42
scaled the readonly pool down to 1 replica
began continuously looping updates to set that column to unixepoch("subsecond")
had another client continuously pulling this value from the readonly K8s service endpoint with level=none
scaled up the readonly pool to to 4 replicas, then a couple minutes later up to 7 replicas, reusing the same PV from before with the older data
verified no output from step 5 showed the stale value 42

I assume this shouldn't be a surprise to you but I just wanted to prove it to myself. :)
I'll update the chart version to 1.0.0 and merge to master in a bit.


FR: [Messaging] Add option to retry sending or expose Retry-After header

According to https://firebase.google.com/docs/cloud-messaging/admin/errors, clients should honor Retry-After header for messaging/server-unavailable and messaging/internal-error.
However, the MessagingDevicesResponse does not include a Retry-After header.

It is up to the server to decide whether to include that header in the response or not. If it is not present in the response, the client may retry immediately.

Thanks, @hiranya911 !
I don't know how I can check if the header exists. The formatted response does not mention it. https://firebase.google.com/docs/reference/admin/node/admin.messaging.MessagingDevicesResponse.

I suppose we need to expose that header in the response somehow. Will look into it.

Hello @hiranya911,
Is there any update on it?
I can't find anything in the docs or in the code.
Cheers

@sdomagala there hasn't been much direct progress on this. However, we recently re-implemented how the SDK makes HTTP calls (see #280 and all the PRs referenced therein). This makes it easier for us to support this requirement.

@hiranya911 Hello Firebase Team.
Is this thread related to my newly created issue here
Thanks!

Just sent #518 for review, which implements the retry framework for the SDK.

As of #556 the SDK automatically retries on 503 errors (up to 4 retries), and it also honors the Retry-After header when available. Otherwise it would use regular exponential backoff.

For this would it also make sense on a 500 error as well?

Hi, May I know does the retry also apply to QUOTA_EXCEEDED?
I have lot of messages still failed due to QUOTA_EXCEEDED when using FCM admin SDK 9.2.0.
Note: I have already consulted Firebase Support about QUOTA_EXCEEDED issue, and they told me I need to handle retry-after header.


Add a reference to the Trainer on the LightningDataModule

What does this PR do?
Fixes #3682
cc @nateraw
Before submitting

[x] Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
[x] Did you read the contributor guideline, Pull Request section?
[x] Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
[ ] Did you make sure to update the documentation with your changes?
[x] Did you write any new necessary tests?
[x] Did you verify new and existing tests pass locally with your changes?
[ ] If you made a notable change (that affects users), did you update the CHANGELOG?

PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃

Hello @ananthsub! Thanks for opening this PR.

In the file tests/core/test_datamodules.py:

Line 257:1: E302 expected 2 blank lines, found 1

Do see the Hitchhiker's guide to code style


Build Progress can't reach 100%

Hi, I am using electron and when the file is merge, build progress can not reach 100% even after I use easydl.destroyed() and rerun easydl. But, it will be success if I redownload it with cleaning the chunk first.
Any thought on this?

Ups, sorry, my bad.. I am not focused. The build progress is print just only before 100, but, it's not an error.


python310Packages.shtab: 1.5.5 -> 1.5.6

Description of changes
https://github.com/iterative/shtab/releases/tag/v1.5.6

Things done

Built on platform(s)

[x] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

[ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
[ ] Tested, as applicable:

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

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

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module
[ ] (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes

[x] Fits CONTRIBUTING.md.

Result of nixpkgs-review pr 198567 run on x86_64-linux 1

4 packages built:

dvc
dvc-with-remotes
python310Packages.shtab
python39Packages.shtab


Headers are not included when adding a redirect

Describe the bug
We'd like to set a Strict-Transport-Security header for all responses. This works fine by adding it in globalHeaders except for routes that include a redirect, for those routes the headers are not included.
Adding the headers to the route specifically does not work either.
To Reproduce
Just create a static web app with the following config:
{
  "routes": [
    {
      "route": "/redirect",
      "redirect": "https://example.com",
      "statusCode": 302
    }
  ],
  "globalHeaders": {
    "Strict-Transport-Security": "max-age=63072000; includeSubDomains; preload"
  }
}

Try issuing a GET request for /redirect. Notice how the Strict-Transport-Security header is not included in the response.
Expected behavior
I would expect all headers in globalHeaders as well as those in the header block for the route to be included even when issuing a redirect.

Hi! Thanks for identifying this issue! We will make a fix to support global and route headers on redirects and will update this thread when we do! Thanks for your patience and sorry for any inconvenience this may cause!


A plea to include binary API required by socket.io into NodeJS core.

I just posted this request to add the binary API required by socket.io into CORE node code -rather than force every Windows node app user to have an outdated SDK and compiler installed.
https://github.com/joyent/node/issues/8005
Most end users

(a) don't have admin privs
(b) are not developers or experienced IT techs
(c) don't have Visual Studio installed
(d) If they do use Visual Studio they ditched Visual Studio 2010 long ago.

To make matters worse .. attempts to download the SDK from Microsoft failed with MS having the wrong links.
Once again I'm hitting this ten pound gorilla that refuses to go away. IMHO, I think requiring every node end user lib to tick all these boxes if their app wants to use socket.io is the wrong solution - the binary API belongs as part of node code so the compiler/SDK problem is no longer.

@tohagan I think this issue is a little premature.
Right now there are no signs of ws moving to node.js core. Whether that happens or not is out our hands. If that happens at some point we can revisit this issue and start using the core provided module instead.
The native code in ws is anyway optional and there is a fallback js implementation that will be used if the compilation failed, so there are no other usage drawbacks than some performance loss.


Capital and always same wording

Capital E-mail -> E-Mail
and always same wording E-Mailadresse

ping

updated

This PR is not mergeable. We cleaned the language files.

ok - then i close this PR and open a new one: #541


Test against Ruby 2.1 nad 2.2 on Travis CI

It works fine for me.

I rebased my commit with the current master.

cool thanks


Add tasks.dart to interface with Google Tasks

Wrote this for a script I'm writing, figured I'd share it with a pull request. It all works!

Thanks!

I'm going to be opening a few of these over the next few days as I build out what I need for a project I'm working on. Thanks for this package!

That would be great.
Glad it is useful to you.


Issue #273 sprite culling

This fixes the issue #273. It also provides two added helper math functions for params float[] args of min & max. Along with an optimization to only calculate bounding box upon changes to worldMatrix or sprite size.

@dklompmaker I gave this PR a try and my suspicions are confirmed: this code does the same as the old one. I can still repro the bug even after.
I will take a look and try to fix it now that I started debugging it 😄

Typical triangle inequality situation (https://en.wikipedia.org/wiki/Triangle_inequality).
‖x + y‖ ≤ ‖x‖ + ‖y‖

@Kryptos-FR Yes exactly. So it broke down as soon as there was some rotation.


Add “—custom-extensions” option to allow user to prettify files with extensions that are not listed in Prettier supported languages extension

User can specify custom extensions .eslintrc or .babelrc with —custom-extensions option and they should be prettified.
Dot-file extension can’t be matched with path.extname util and I added logic to match all characters after last dot in the file path for custom extensions.
README was updated with new option description.
I add .babelrc custom extension in precommit npm script. You can try to change .babelrc file (add extra spaces somewhere) and then commit changes, .babelrc will be prettified.

Why would that not work with --pattern .babelrc?

For .eslintrc, I believe that is deprecated and you should use .eslintrc.json.
For .babelrc, we should add that to the supported files here: https://github.com/ikatyang/linguist-languages/blob/962406b80ee2b6c248dd1446da3f2c3cb04ee177/data/json.json#L23-L30, Prettier respects this information, and will format .babelrc files as JSON without any configuration!

I believe that is deprecated and you should use .eslintrc.json.

We have a mono-repo and I had an issue a half year ago, when I tried to use .eslintrc.json instead of .eslintrc. It was on previous major version of eslint and babel-eslint, but still.

we should add that to the supported files here:

The problem is that JS-ecosystem grows or changes faster than you can create pull requests on this list.
These file extensions I added as custom for our project

With this .prettierrc file all my custom extensions are prettified when I run

prettier --write "**/*.{js,json,md,prettierrc,eslintrc,lintstagedrc,firebaserc,babelrc,sublime-project}"

but are not prettified when I change some of them, stage and run
pretty-quick --staged

This is inconsistency and it should be fixed on pretty-quick side and not on prettier, IMHO.

It's just easier and more reliable way for users to specify their custom file extensions as additional argument than to submit a new pull request to Prettier supported languages list each time framework or library they use were changed and new dot-files were added or removed.

So uh, as an interested observer… shouldn't pretty-quick be consulting the prettier overrides list when it figured out what files to reformat?
Like, I'd expect that if I add a custom thing to overrides, then pretty-quick would just automatically pick that up.

shouldn't pretty-quick be consulting the prettier overrides list when it figured out what files to reformat

In theory it is possible with glob npm-package. We use it in our merge-check scripts in Bitbucket Pipelines. You can read .prettierrc file, find "overrides" property and and loop through items, checking if input file path matches its glob-pattern. The problem could be with dot-files as there is a flag in Glob how it processes the pattern depending on dot is true or false, and potentially this way of fixing the problem will require one more cli argument to work - --accept-dot-files.

So uh, as an interested observer… shouldn't pretty-quick be consulting the prettier overrides list when it figured out what files to reformat?

It actually does, via prettier.resolveConfig. The issue is that this acts more like a filter, as pretty-quick only runs on known file extensions to begin with, and overrides are for changing rules based on file paths, not really for supporting new languages.

@serhiipalash of the command you posted:
prettier --write "**/*.{js,json,md,prettierrc,eslintrc,lintstagedrc,firebaserc,babelrc,sublime-project}"

Already supported by pretty-quick:

js
json
md

Not currently supported:

.prettierrc - Not always JSON
.eslintrc - Not always JSON
.lintstagedrc - Not always JSON
.firebaserc - JSON
.babelrc - JSON
*.sublime-project - JSON

The problem is that JS-ecosystem grows or changes faster than you can create pull requests on this list.

I don't that's true, the cost of creating a PR is very low.
I would encourage opening a PR to add .firebaserc, .babelrc and .sublime-project here, as they're all commonly used, and always JSON files. Once they're in here other people won't have to configure anything (zero-config philosophy) (make sure .sublime-project goes in extensions).
For .prettierrc, .eslintrc and .lintstagedrc, I would highly encourage adding a .json` extension to those files (supported by all three tools). This way not just Prettier knows that they are JSON files, your editor will too.

Going to close this as I'd prefer we took the approach outlined above.


Rename files/function in e2e tests for better understandability

Description
Please include the following:
This PR renames files/function in e2e tests for better understandability
How can this be tested?
via make test/e2e
Checklist

[x] Unit tests have been updated/added
[x] PR is labeled accordingly with a single label
[x] I have read and understood the contribution guidelines

Codecov Report

Merging #2104 (f9ee6c1) into main (e732cff) will not change coverage.
The diff coverage is 100.00%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
@@           Coverage Diff           @@
##             main    #2104   +/-   ##
=======================================
  Coverage   63.24%   63.24%
=======================================
  Files         242      242
  Lines       15376    15376
=======================================
  Hits         9725     9725
  Misses       4909     4909
  Partials      742      742

Files Changed
Coverage Δ

src/api/v1beta1/dynakube/dynakube_types.go
100.00% <ø> (ø)

...rc/controllers/csi/driver/volumes/volume_config.go
75.47% <ø> (ø)

...ntrollers/dynakube/oneagent/daemonset/arguments.go
100.00% <100.00%> (ø)


Constraint based type inference

This PR proposes a constraint-based global type inference algorithm for egglog. More importantly, it proposes a core IR framework for writing rule analysis and transformation passes, which facilitates future development.
In terms of type inference:

The type inference algorithm is inspired by Souffle and other Datalog compilers' design: it goes through each query and action atom, generates respective type constraints, and uses a simple fixpoint-based solver to solve type constraints. This design is declarative (one can inspect generated constraints) and scalable (one can add new type constraints to handle new features).
This PR fixes a number of issues with the current type checker, e.g., #196, #133, #113, and potentially #80.
The type inference algorithm supports both polymorphism and ad hoc overloading: for example, it supports both polymorphic functions like vec-add : vec<a> -> vec<a> -> vec<a> and overloaded functions like add, which has several definitions such as f64 -> f64 -> f64 and String -> String -> String.

In terms of the core IR:

The old typecheck.rs is refactored into several orthogonal passes:

A lowering pass that transforms a Rule into CoreRule,
A congruence pass that gets rid of redundant atoms,
A canonicalize pass that removes equality check and "canonicalizes" variables,
A type checking pass.
Another lowering pass to Program (not currently implemented).

A CoreRule has two parts: Query and Actions. Query uses a conjunctive query--based IR, while Actions uses an imperative SSA-based NormAction, which is currently used in desugaring. The goal is to gradually phase out NormExpr once desugar.rs is refactored into several compiler passes over core IR, so that we have a unified internal IR for query analysis and transformation.

I had a discussion with @oflatt about the differences between the proposed conjunctive query-based IR and the existing NormFact IR. The goal of the proposed IR is to provide a flexible representation where we can develop a series of nano-passes and analyses that progressively transform the query into an efficient form.
However, this is lacking in NormFact.
To summarize my ideas, here are some advantages of the proposed IR in my opinion:
It is declarative and unordered.
The existing NormFact IR enforces an order between atoms, as is shown in #196. For example, consider the rule
(rule
  ((= x y)
   (= y 1))
  ((R x)))

The query corresponds to NormFact
AssignVar x y
AssignLit y 1

which is not executable and causes egglog to panic.
In the proposed IR, it will first be lowered to
(rule
  ((value-eq x y)
   (value-eq y 1))
  ((R x)))

After that, a canonicalization pass will substitute x with y in the rule, and then substitute y for 1:
(rule
  ()
  ((R 1))

One way around this is to eliminate the ordered-ness of the NormFact and use ConstrainEqLit, etc. to model an equality check similar to value-eq. But even with this, NormFact is still not a good target for query optimizations. See the second point.
The proposed IR is freer in the sense that it gives you a bigger space of expressible programs.
value-eq, which is a primitive for equal checks, subsumes all of AssignVar, AssignLit, and ConstrainEq and is more general. Consider a simple rule
(rule
  ((= x (Num 0))
   (= z (Mul x y)))
  ())

In NormFact it looks like this
(AssignLit __var 0)
(Assign x (Num __var))
(Assign z (Mul x y))

This indirection introduced by variable __var complicates query evaluation, since now without looking at the context we don't know if (Num __var) is a look-up (which can be further optimized) or scan. The point is that because of the lack of flexibility of NormAction, there is no way of inlining 0 in place.
Consider another example:
(rule
  ((= (Num y) (Num x)))
  ())

In NormFact it looks like this
(Assign z1 (Num y))
(Assign z2 (Num x))
(ConstrainEq z1 z2)

Again, there is no way of expressing the same query efficiently in NormFact, because the IR disallows "inlining" z1 and z2.
In contrast, in the proposed IR, the above NormFact looks like this:
(Num y z1)
(Num x z2)
(value-eq z1 z2)

A canonicalizing pass will substitute z1 with z2 and give you
(Num y z1)
(Num x z1)

which directly corresponds to an efficient join.
Oliver had a good point that for proof generation, you may need to handle literals specially, so with NormAction one can assume all the arguments to functions in Assign is a variable, which can be handled uniformly. However, because the proposed IR is more expressive, it captures programs expressible in NormAction, so one can have a "de-canonicalization" pass that replaces every argument with a variable and generates a bunch of (value-eq var val) atoms. Then, after proof generation is done, another canonicalization pass can be used to eliminate these unnecessary value-eqs
A uniform view of atoms is useful for query compilation.
(This point is less relevant to NormFact IR but I think it's very cool.)
In the proposed IR, a query is just a vec of atoms, which abstracts over both functions and primitives. This uniform view is useful for further optimizations in query compilation. For example, consider
(rule
  ((R x y z)
   (= (+ x y) z))
  ())

In the proposed IR, it corresponds to two atoms
(R x y z unit)
(+ x y z)

The only difference between + and R is that + enforces a functional dependency. One can therefore generate a query plan as
for x in R.x
	for y in R[x].y
		for z in Isect(R[x,y].z, +[x, y].z):
			output [x, y, z]

The current query compiler, however, will go through each z in R[x, y] and do the check afterward.

Update: All the tests are passing! Some big TODOs left (maybe for future PR):

[] #196, #133, #80: Since the errors are thrown by NormFact, we have to refactor or remove NormFact to fix this issue.
[] Refactor type inference for actions: Currently this is done by reusing the existing action checker and is not integrated into the proposed framework.
[] Refactor query compilation: The proposed IR does not split up atoms into primitives and functional atoms, while the query compilation and evaluation pass does. To be compatible with query compilation I introduced func() and filters() methods that simulate what was the atoms and filters fields. This is however inefficient. The principled way to do this is to also refactor query compilation to reflect this unified view.

As far as I can tell, this PR doesn't replace typechecking.rs- which means that there will still be two typecheckers if we merge this. Is that right?

Is the type information from typechecking.rs used in other places? If this is true then you are right.
Are you okay with merging it while two typecheckers are present? If not, I'm happy to put more work into this PR, but this might make code review more complex than splitting into several PRs of smaller scopes.

Typechecking information is used by my terms and proofs encoding unfortunately.
I think ideally, there would be just one type checker and it would happen just after desugaring but before the program reaches the back-end. In my mind sending a type-annotated AST to the back end would be best.

Merging with two typecheckers would raise the question- which one are we going to keep in the future?

Yeah, I understand it's quite difficult to avoid conflicts with my branch.
However, I still believe that will some small tweaks we can merge it much sooner than after the eggcc project, which will take a lot of time.
Regardless, as you said this typechecker seems better and it makes the backend cleaner. I think your suggestion of keeping both typecheckers for now is a good compromise. I would just ask that you don't remove the old typechecker and that you do not remove NormCommand/NormFact.

Update: This PR is self-contained and ready for review. The refactoring is not 100% done, but it is passing all the tests which is a milestone. So I propose to consider this PR as is for merging, and I plan to make follow-up PRs. More details:

https://github.com/egraphs-good/egglog/pull/219#issuecomment-1701390640 is still true.

Compilation of actions is still done using the existing code, not the constraint-based type inference algorithm
The mentioned issues still exist- need to remove the front-end type check to do this
gj.rs and other places still use query.funcs() which is slow.

Right now type error generated by the constraint system is a bit hacky. This is because constraint solving is implemented in a very general way (which is good) while the error information we need is very specific. I plan to refactor this by having a generic error type parameter in the constraint system.

TODOs for follow-up PRs (will open a tracking issue for them once this is merged).

Add a renaming pass to resolve name conflicts/shadowing once and for all
Improve error information for constraint solving.
Add a congruence pass for optimization (so that (= c1 (add a b)) (= c2 (add a b)) will be compiled into one atom)
Refactor type inference for actions.
Refactor the execution for actions to work over NormAction
Use an opaque type instead of AtomTerm for fn get_constraints in the primitive function interface.

cc @mwillsey @oflatt

Are the new queries any slower now that we are not doing congruence?

Oh, it’s congruence on query atoms not congruence in rebuilding’s sense.
This is just an easy and obvious optimization pass.
I put it in TODOs because the old type checking code does this optimization.
On Mon, Sep 18, 2023 at 12:20 PM Oliver Flatt @.***>
wrote:

Are the new queries any slower now that we are not doing congruence?
—
Reply to this email directly, view it on GitHub
https://github.com/egraphs-good/egglog/pull/219#issuecomment-1724231440,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ADEKPFDOXRTHOGLY7AXGOOLX3CNGRANCNFSM6AAAAAA4DKMNJI
.
You are receiving this because you authored the thread.Message ID:
@.***>

Before I do my review, I just want to say amazing work @yihozhang and we make a great team! This is a big moment for egglog.


[Bug] Keychain Access does not show updated keychain entries

TablePlus keychain entries don't show up in GUI tool Keychain Access after being updated though accessible through /usr/bin/security
This far failed to overcome this issue with both security add-generic-password -T and security set-generic-password-partition-list -S options

They were there, just under name 'com.tableplus.TablePlus' instead of 'TablePlus' as expected
Fixed in 0.0.2


Refactored features, extractor classes; streamlined data proc and model training; changed API

First things first: There are A LOT of changes in this PR, and I'm not quite done, so many apologies in advance for this incomplete monster.
Here's the rundown:

I refactored all features so that they're sklearn-style transformers, with key methods of .fit() and .transform(). The underlying logic that converts sequences of blocks into 2D arrays of numeric features has been moved into and optimized in cython (with the exception of CSS features, which rely entirely on regexes). There are a number of benefits of switching to Transformer classes: easy combining, parallelizing of feature generation plus joint parameter optimization via sklearn.pipeline.FeatureUnion, interoperability with other sklearn features such as StandardScaler for standardizing feature values by mean and standard deviation, plus nice inherited behaviors from the sklearn base classes such as pretty-printed instances. I moved them into their own subpackage, to be tidy.
I refactored the core ContentExtractionModel into an sklearn-style classifier Extractor with key methods of .fit() and .predict(), plus the primary entry point for users of .extract(html, encoding). This class flexibly combines features, a classifier model (ExtraTreesClassifier by default), and a couple other optimizable parameters in a way that is also sklearn-friendly and pickle-friendly. It's easy to use grid search to jointly optimize features (and their individual params), the classifier model (and its params), as well as block weighting and the prediction probability threshold.
I added top-level convenience functions to extract content, comments, and content+comments using pre-trained and -optimized Extractor models. They are loaded lazily from disk once, then cached. Loading is very fast via joblib (bundled within sklearn), although the pickled, compressed model files are surprisingly large (~14MB apiece).
I streamlined much of the data processing and model training code, e.g. by removing seemingly deprecated or unnecessary code, generalizing some functionality, and reducing the amount of hard-coding throughout. I took advantage of sklearn's existing metrics for, e.g. accuracy and F1 score, and applied them to block-level binary classification performance. I also added jaccard similarity and the existing damerau levenshtein to evaluate extracted vs. gold standard text content. I removed plotting functionality because it seemed like a thing best left to developers, but could totally put diagnostic plots back in.
I cleaned up pep8 violations, improved documentation in sphinx-compatible Google docstring style, and laid a bit more groundwork for Py3 compatibility. On that note: I did not touch any Blockifier-related code, because it's so complicated and because you have other packages (?) that rely on it as-is. blocks.pyx seems to be the last bastion of Py3 incompatibility — in unicode vs. bytes issues, of course. It's also still the slowest part of extracting content from html, by far.

There are still a few to-do's:

I've not finished porting tests over; temporarily, I'm skipping the two problematic test cases in unittest.
There's still a bit of a bytes and unicode mishmash, particularly in the data processing code. Nothing appears to be broken, but I'd feel much better working entirely in unicode under the hood. This will probably wait until blocks.pyx gets sorted out, which I probably can't do on my own. I tried reading this but got horribly lost.
I've not settled on final extractor models (although pretty good pickled model files are included in this PR) because performance characteristics are slightly different. I'm optimizing by block-level classification performance of F-beta score (e.g. F1) using sklearn's grid search + cross-validation.
I've not yet done a full and thorough runtime and classification performance comparison. Initial tests have shown feature generation to be about twice as fast (maybe owing to the built-in parallelization of FeatureUnion). Also, I've found my best models to have higher recall but lower precision than previous versions (see below), but as I said, I'm still tweaking things. Would appreciate your feedback on this!
The "public" API has changed, as has the workflow for training new models. Documentation has not yet been changed, pending your feedback. Btw, I'm happy to work towards better backwards compatibility, if you think that's important.

(purple is the old model, blue is the new model)
Apologies again for this huge PR, one change kept leading into another... Let me know what you think about all this.

Whoa, this is a big PR.. I'll hopefully have time to look at it later in the week.  One immediate thought without looking at the code is that a complete change of the API is likely a no-go for Moz since we have a fair amount of legacy code depending on the current API, warts and all.  To give you some idea about the difficulty of changing the API completely, Moz has four different projects in prod that depend on dragnet across different teams so it'd be a rather large effort to swap it out completely and one that is unlikely to get support.

@matt-peters I thought you'd said only the blockifier API was relied upon elsewhere — my bad. For what it's worth, the other projects could simply pin their dependency version to dragnet==1.1.0 until they felt like porting over, but since I don't know how entangled everything is, I don't know how much work the eventual porting would be. I'll see what I can do to emulate the old API. Just to be clear: Are they only using the basic API, e.g.
from dragnet import content_extractor
content_extractor.analyze(html_document)

?

Sorry if I indicated earlier that the blockifier was the only interface used.  Unfortunately it's impossible for me to give a complete list as I'm no longer at Moz and don't have access to the legacy code (perhaps @dlecocq or @sistawendy could add some more detail?).  So this is just from memory, but it likely includes:

the blockfier interface, BlockifyError, lower level details of the Blocks class, the dependence on lxml as the HTML parser
content_extractor, content_comments_extractor, content_and_content_comments_extractor: the analyze method (with various options, which were add at some point for a use case).  It's also possible that we are using analyze_from_blocks or the various other lower level methods in these classes too as a common pattern is to move the lxml parsing from dragnet elsewhere but re-use the parse tree through the lower level methods.

As far as I'm concerned all the data processing and model training stuff isn't important to maintain in it's current form (as long as functionality is maintained somehow) since this is all done offline and not at run time.

Hey @matt-peters , just checking in. I was waiting to hear back re: API changes before doing any more work on this, but: I'll try to do and submit a full comparison of extraction performance and runtime between this and the previous version sometime this week.

Sorry about the silence on this one.  I'm having a hard time reviewing this due to the massive size.  Is there any way to split this into smaller PRs?
Also, we'll get you an answer about potential API changes soon.

Hi @matt-peters , let me see what I can do about splitting this into smaller PRs... Changes to the features, and then the extractor, arose from changes I'd wanted to make to the model training. So, maybe I'll start from the bottom and work my way up rather than the other way round. If it's all the same to you, I'll leave this PR as-is and submit others piece-by-piece.

OK we've decided to pin internal Moz dependencies on the current pip version so can move forward with API changes.  I'll hopefully have a chance to review the API changes later today so we can at least fix those before breaking it into pieces and merging.

hey all. We're (@pakelley) also interested in helping port this library to Python 3. In trying to find a roadmap or any existing efforts, came across this PR. What's the status of this? If the idea with the effort is to still break it apart, we'd like to contribute where we can. Appreciate it. 😄

Hi @nehalecky , I've been totally diverted by other projects, so haven't yet had a chance to revisit this PR. Help would definitely be appreciated! These changes arose from my wanting a better API for training / applying the extraction model, and opting to follow scikit-learn's lead. Code and documentation cleanup, as well as steps toward Py3-compatibility, were just bonuses.
FWIW I think blocks.pyx is the biggest blocker to making dragnet Py3-compatible.

Hi @bdewilde, really appreciate the quick response and the details. I also commented on a collaborative effort for the porting in #26 to see what engagement we can get there. Look forward to the next steps, soon. :)

Closed in favor of #52.