Update discipline-munit to 2.0.0

About this PR
📦 Updates org.typelevel:discipline-munit from 1.0.9 to 2.0.0 ⚠
📜 GitHub Release Notes - Version Diff
Usage
✅ Please merge!
I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.
If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention me in the comments below.
Configure Scala Steward for your repository with a .scala-steward.conf file.
Have a fantastic day writing Scala!

⚙ Adjust future updates
Add this to your .scala-steward.conf file to ignore future updates of this dependency:
updates.ignore = [ { groupId = "org.typelevel", artifactId = "discipline-munit" } ]

Or, add this to slow down future updates of this dependency:
dependencyOverrides = [{
  pullRequests = { frequency = "30 days" },
  dependency = { groupId = "org.typelevel", artifactId = "discipline-munit" }
}]

labels: test-library-update, early-semver-major, semver-spec-major, version-scheme:early-semver, commit-count:1

Done in #445.


Make CameraCalibrator choose calibration based on Camera ID, not file type

Right now, CameraCalibrator constructs the calibrator based on the type of the file (as read by the EventSource).  In the future we will have the raw data of multiple cameras in the same format, so we should really choose the calibration type from the camera ID (like "NectarCam") which you can get from the SubarrayDescription that is loaded from an EventSource currently.
This may require more of a design change, since right now we "calibrate" Monte-Carlo differently than real data (which really is not a good way to work, since in the end they must be identical)

TO be more clear, it should be something like this:
#This:
calib = CameraCalibrator(subarray_description=event.inst.subarray)

# rather than this:
calib = CameraCalibrator(event_source=source)
``

Internally it may set up multiple calibrators, covering all telescope types (it could use subarray.telescope_types (which is something like ['SST-ASTRI:ASTRICam', 'LST:LSTCam', 'MST:FlashCam'] to decide which ones to use.  The SubarrayDescription also allows mapping of tel_id to type, via ids = sub.get_tel_ids_for_type("SST-ASTRI:ASTRICam")

I think this got fixed in #1228


Update UB <IP_ADDRESS> breaks backwards Palace of Ice save compatibility

I can't leave more details atm but upgrading from .12 to .14 makes all saves in the Palace of Ice folder incompatible.
Restarting the whole campaign fixes the problem [but it's far, far from ideal]. Saves from later on in the game trigger the "issue with mods" error dialogue.
Reverting the game to .12 is impossible from .14's UB interface. Needs to be d
Palace of Ice.zip
one externally.
Reverting to a version .12 fixes the issue.
Attaching all save in the campaign. "ShamboCoop.sav" is the latest.

Have you read the changelog before update?

I always do - did I miss something so macroscopic I didn't notice? Recently the changelog became also the what's new and what's next - could you please elaborate?

Even after respeccing the character as a human, it still breaks the load menu if at least 1 save uses a previous version.
Is that the expected behaviour?

I don't get it tho - I'm not using beta releases. This change's quite gamebreaking! 😅

it still breaks the load menu if at least 1 save uses a previous version.

From the release notes:

RESPEC any Malakh race hero or save before update

I see. So it was intended to be a breaking change. Of course I can't be respeccing previous savegames of the whole campaign so I will need to move them elsewhere - even if they're not actively loaded they do prevent the game from working correctly.
Thanks for the clarification, I'm closing this as won't fix/works intended.


Add example for serving a full stack web application, combining static content with rest api

In our team we would like to use Pistache.io as a lightweight web server on some embedded device with very tough hardware requirements, serving a full stack web application as IoT UI for the embedded device. That means serving both a REST API and the corresponding SPA web client as static content via Pistache.
The static content should be available, e.g., under the URL /content/ and the REST API under /api/v1/. How would I do this in Pistache with wildcards such that requests for /content/* are mapped to some local folder while all requests for /api/v1/ go to endpoint handler methods?
Can I somehow use the functions in the namespace Routes for this? I haven't really worked with Pistache before and http://pistache.io seems to be down, including all the documentation.
Any code example for the above full stack use case would be highly appreciated. Maybe even add it to the repo's examples folder?

As far as I'm aware there is no directory serving support in Pistache - It's not really in scope for the project.
So you'd need to draft that yourself, which should be too hard as all the HTTP support is present (MIME, Streams, etc.).
Your main concerns would be how to respect filesystem boundaries, I.E.: If you wanted to serve the subtree of /var/www/static via htttp://my.url/static how would you stop me from making a request such as htttp://my.url/static/../../../etc/passwd to retrieve /etc/shadow or if you accept uploads how would you stop me from uploading my own /root/.ssh/authorized_keys.
Having said that;
There are numerous other similar risks to running a directory service that projects such as nginx and apache have taken over a decade to identify and solve.
As I've mentioned in your previous issues on the subject of using pistache as a primary webserver for an IOT product - This project is not ready for that, and there are far too many risks that you'd be exposing yourselves too.
We haven't even released a V0.1 and yet you're trying to treat it as a production ready V1.0. Please re-evaluate - we don't want to get blamed for your product having a severe security breach. Are you certain you can't put a lightweight service such as nginx or lighttpd in front?

I put all of my static content into a TAR file, embedded that into my binary via a linker trick, and use libtar to index the contents at process startup.  I then have one route which serves static content using a wildcard, and it handles getting the bytes from the process image.  I do not have directories in my static content, so I do not need to worry about path traversal.  This works well for my use case.  I only have 5 static files (main page html, css, js and two supplemental CSS files).

Hi Dennis,
That's a unique solution, I'm not sure I would have gone about it that way, but it's definitely a solid way of serving a small amount of content securely.
While I think your solution could be useful to @svdHero I doubt he has the RAM necessary to support that.

Thank you guys for all your explanations. Let me quickly add:

We haven't even released a V0.1 and yet you're trying to treat it as a production ready V1.0.

That is - if I may say so - a false assertion you're making here. In fact, the contrary is the case. I am a responsible and careful developer and that is exactly why I am NOT treating it as production ready but asking all these questions ahead of time BEFORE I use Pistache in production.
Ever since @dennisjenkins75 pointed out the memory leaks to me in #226, I have been following progress in #399. I will only start considering Pistache for IoT, once #399 is closed and Pistache has matured a bit. However, I want to play with it now in order to get a better understanding if it could be a viable option in the future.

we don't want to get blamed for your product having a severe security breach.

I would never do such a thing. I take full responsibility for the tech stack choices I make in my own products. I do understand the intention of your statement, though. :wink:

Are you certain you can't put a lightweight service such as nginx or lighttpd in front?

I will give it a try. However, our embedded system only has 64 MB of RAM and for historical reasons there is already a local C# GUI running under Mono which is quite memory hungry.
As I (similar to @dennisjenkins75 ) probably will have only a small number of static files for my SPA, I might also spell out all routes to the individual static file like so
https://github.com/oktal/pistache/blob/1df04a35cd54f476dc788c0175f37395df701f7d/examples/http_server.cc#L140-L146
That would be a viable solution, too, wouldn't it? Or am I missing something?
Thanks a lot again for all your work and effort to keep this project alive and bring it to production ready (some day). :+1:  I really appreciate it.


RFC: Github-CDEvents Integration

This RFC describes
an approach to integrate Jira with CDEvents using using webhook-adapter.
and the list of Github-Webhook event types mapping with CDEvents.
This RFC is related to this task cd-event-github-plugin

Thanks for this, I left a few comments. Let me know what you think. The idea in cdevents/webhook-adapter#2 is to create a plugin for the existing webhook adapter, not to create a new one. You could use the existing gerrit plugin as a starting point: https://github.com/cdevents/gerrit-translator/

@afrittoli  i have just modified the PR and tried to modify and simplify it as i could and got inspired by the https://github.com/cdevents/gerrit-translator/. Let me know what you think about. This could be a good starting point.


Defining the name of a page

I've been playing around with this module a bit and so far it works really well. There's one thing that's bothering me though.. That's the name property. It looks like it's up to the initiator of the view to decide what the name of the view should be. For instance, on line 45 of HomePage.js in the twitter example:
{
  goToTweet: function(tweetData) {
    this.props.toRoute({
      name: "Tweet",
      component: TweetPage,
      data: tweetData
    });
  }
}

The name is being defined there. What if the app has several places that initiate this? Would I have to add this all over the place? Can it be defined from within the page view itself, too?

You could accomplish this like so:
{
  goToTweet: function(tweetData) {
    var TweetPage = require('./pages/tweet')';
    this.props.toRoute({
      name: TweetPage.title,
      component: TweetPage.component,
      data: tweetData
    });
  }
}

with pages/tweet.js exporting:
module.exports = {
   title: 'Tweet',
   component: myTweetComponent
};

Alternatively this works as well:
{
  goToTweet: function(tweetData) {
    var TweetPage = require('./pages/tweet')';
    this.props.toRoute({
      name: TweetPage.displayName,
      component: TweetPage,
      data: tweetData
    });
  }
}

Just ensure you have the displayName property defined to your desired title on within your React.createClass.

Satisfying enough. Thanks.


[Enhancement] Debug further on higher Limit for Cache TTL

We should have a higher limit on cache for TTL not only to keep a logical limit but also due to security reasons.
DAB users shouldn't be allowed to use INT.Max as the upper limit which is the situation currently.
In this issue, we should explore with developers using DAB and get a logical conclusion for this limit.

Can you add your reasoning for why Int.Max is a problem and why a higher limit is not?


CSS.escape not always exposed in WebKit Nightly

It seems that an internal optimization / bug in WebKit/JavaScriptCore can cause CSS.escape === undefined even though the polyfill is properly loaded and executed.
This can be observed in Safari on El Capitan an WebKit Nightly. The behavior is not reproducible consistently. On my system it always occured on cold cache and every 3rd or 4th reload.
If you add console.log(CSS.escape) right after the polyfill's function expression, the function is always available globally. The same is true for explicitly assigning the function to the global space using root._css_escape_webkit_fun = CSS.escape = function(value){ … }. This leads me to believe some optimization heuristic is going haywire.
I'm aware this is not a problem of this project, yet this is the first - and only - situation I've seen this happen. I have not opened a WebKit bug for this, mostly because I don't even know the terms to search for existing reports…

Do you have a test case I can use to try to reproduce this?
Can you report this on https://bugs.webkit.org/ anyway? The team will triage it, don’t worry.

sure! testcase and WebKit issue 149175

Closing in favor of the WebKit bug ticket. Thanks!


Project:  TimeForAJoke - A small command bot which loads a developer joke from the API

Project name
TimeForAJoke
Description
A small command bot which loads a developer joke from the API "https://backend-omega-seven.vercel.app/api/getjoke" on the command "devJoke" and outputs it in an adaptive card.
With each command a new joke is loaded from the API.
Project Repository URL
https://github.com/SomebodyToLove1337/HackTheJokesOut
Project video
https://github.com/SomebodyToLove1337/HackTheJokesOut/assets/68236595/4ab4050f-c61a-4de2-98d7-b4c854b984c2
Team members
SomebodyToLove1337, maxhe87

Thank you for your project submission @SomebodyToLove1337, I'm excited to check out your project details! Meanwhile, would you and your teammate be willing to fill out our hackathon survey? That'd help us learn from your hackathon experience and improve it next time: https://aka.ms/hack-together-teams/survey

Such kind words, thanks @SomebodyToLove1337! 🙏 So great to hear you've got an opportunity to learn so much and submitted your project successfully! Looking forward to meeting again in our next hack! 👏🤗


The server contribution docs say to rebase off master, but we encourage merging instead

Item 9 of the Server Workflow docs says:

Respond to feedback on your pull request and make changes as necessary by committing to your branch and pushing it. You might need to rebase your changes if another pull request creates conflicts.

We can instead of have something such as:

Once a PR is submitted it's a best practice to avoid rebasing on the base branch or force-pushing. Jesse, a developer at Mattermost, mentions this in his blog article Submitting Great PRs. When the PR is merged, all the PR's commits are automatically squashed into one commit, so you don't need to worry about having multiple commits on the PR.

Maybe this should go in a different spot altogether, since it affects every repo, and not just the server repo. @justinegeffen Any thoughts?

Hi @mickmister, I think the Code Review docs will be a good place to have something which you mentioned. Let me know if that sounds good and I will help make the changes :)
cc @justinegeffen

Yeah that's a good idea, @haardikdharma10. I'm happy with that. :)


brew cask install fails

 ▲ ~: brew cask install keepingyouawake                                                                                (⚓ ) 3h
==> Satisfying dependencies
==> Downloading https://github.com/newmarcel/KeepingYouAwake/releases/download/1.4.2/KeepingYouAwake-1.4.2.zip

curl: (6) Could not resolve host: github.com
Error: Download failed on Cask 'keepingyouawake' with message: Download failed: https://github.com/newmarcel/KeepingYouAwake/releases/download/1.4.2/KeepingYouAwake-1.4.2.zip

macOS Hight Sierra 10.13.1
macbook pro 2017

Might have been a temporary problem. I'm not experiencing these kinds of errors here:
==> Downloading https://github.com/newmarcel/KeepingYouAwake/releases/download/1.4.2/KeepingYouAwake-1.4.2.zip
######################################################################## 100.0%
==> Verifying checksum for Cask keepingyouawake
==> Installing Cask keepingyouawake
==> Moving App 'KeepingYouAwake.app' to '/Applications/KeepingYouAwake.app'.
🍺  keepingyouawake was successfully installed!

Maybe just try again today to see if it works.
In general, if there are problems with the Homebrew Cask please have a look at https://github.com/caskroom/homebrew-cask because I'm not the provider of the Cask for KeepingYouAwake, but I'm glad it's there 😃

@newmarcel oh, it was a FYI more than anything. I installed from the release.  Thanks for the project


Icon Size Mis-match

Environment
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.149 Safari/537.36
URL
https://penguin-stats.io/result/item
Steps to reproduce
Sprite image has been a major issue before because whenever a new item has added, the browser cache will still be serving the old item sprite despite it doesn't have icon for the new item.
This bug has been reported repeatedly (ref NGA.cn comment)
What is expected?
The sprite image should render properly and consistently
What is actually happening?
The sprite image file on CDN have to be updated and using a new name every time if there's a change in item data, and the rendering will not be consistent since new data will come with the requirement of a latest sprite image.

This shall be fixed when we add versioning support for our data.

The cache control header for https://penguin.upyun.galvincdn.com/item_sprite.png is correct.
cache-control: public, must-revalidate, max-age=691200
etag: "236f2c8c3b27b143393df0104777f43a"
expires: Wed, 20 May 2020 07:52:16 GMT
last-modified: Tue, 24 Mar 2020 02:19:31 GMT

Does upyun allow content purge? (purge will get the image from the origin which the etag will change for a new file)
Or, do we need hashing in the file name to resolve this?

We planned to use hashing in the file name to resolve this, because purging the CDN could still lead problems to the ones who have got old item data but requesting the new item sprite image.
This shall be fixed in the next release v3.0.0.


Custom template layouts

:hand: Many people requests features. Tell us why yours is important to the
community. How does it add value? Why this feature?
Is your request very specific to your needs? Consider
contributing it yourself! Or reach
out to a community member that offers
consulting services.

Request
What feature or change would you like to see made?
template layouts for dental viewer like LEADTOOLS DENTAL VIEWER
ej: https://medicaldemos.leadtools.com/DentalViewer/#/login
Why should we prioritize this feature?
for custom layouts

We have hanging protocols and advanced layout support on the roadmap. If you want to propose something more concrete, or spike the changes yourself, please don't hesitate to open a new issue.


[SPARK-14972] Improve performance of JSON schema inference's compatibleType method

This patch improves the performance of InferSchema.compatibleType and inferField. The net result of this patch is a 6x speedup in local benchmarks running against cached data with a massive nested schema.
The key idea is to remove unnecessary sorting in compatibleType's StructType merging code. This code takes two structs, merges the fields with matching names, and copies over the unique fields, producing a new schema which is the union of the two structs' schemas. Previously, this code performed a very inefficient groupBy() to match up fields with the same name, but this is unnecessary because inferField already sorts structs' fields by name: since both lists of fields are sorted, we can simply merge them in a single pass.
This patch also speeds up the existing field sorting in inferField: the old sorting code allocated unnecessary intermediate collections, while the new code uses mutable collects and performs in-place sorting.
I rewrote inefficient equals() implementations in StructType and Metadata, significantly reducing object allocations in those methods.
Finally, I replaced a treeAggregate call with fold: I doubt that treeAggregate will benefit us very much because the schemas would have to be enormous to realize large savings in network traffic. Since most schemas are probably fairly small in serialized form, they should typically fit within a direct task result and therefore can be incrementally merged at the driver as individual tasks finish. This change eliminates an entire (short) scheduler stage.

Test build #57212 has started for PR 12750 at commit 4bbf429.

Test build #57215 has started for PR 12750 at commit 5d34a64.

Test build #57212 has finished for PR 12750 at commit 4bbf429.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57212/
Test FAILed.

Merged build finished. Test FAILed.

Test build #57215 has finished for PR 12750 at commit 5d34a64.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57215/
Test PASSed.

/cc @NathanHowell, FYI.

Would Guava's Iterables.mergeSorted[T] help out here?

Maybe, but my hunch is that it's going to be slower and won't save much code.

Alright, here's a few ideas that will at least reduce allocations by a bit. Your version with the merge sort is likely better than the insertion sort here but I thought I'd try something different :trollface: any chance you can run it through your benchmark?
I'll put my other comments inline, generally looks good to me.
  /**
    * Combine two sorted arrays of StructFields into a new StructType
    */
  private def compatibleFields(
      fields1: Array[StructField],
      fields2: Array[StructField]): StructType = {
    // perform an insertion sort of the smaller struct into the larger one
    val (bigger, smaller) = if (fields1.length > fields2.length) {
      (fields1.toBuffer, fields2)
    } else {
      (fields2.toBuffer, fields1)
    }

    var biggerIdx = 0
    var smallerIdx = 0
    while (biggerIdx < bigger.length && smallerIdx < smaller.length) {
      val biggerVal = bigger(biggerIdx)
      val smallerVal = smaller(smallerIdx)
      val comp = biggerVal.name.compareTo(smallerVal.name)
      if (comp == 0) {
        if (biggerVal.dataType != smallerVal.dataType) {
          val merged = compatibleType(biggerVal.dataType, smallerVal.dataType)
          // test to see if the merged type is equivalent to one of the existing
          // StructField instances, reuse will reduce GC pressure
          if (smallerVal.dataType == merged) {
            bigger.update(biggerIdx, smallerVal)
          } else if (biggerVal.dataType == merged) {
            // do nothing, the bigger struct already has the correct field
          } else {
            // we can't reuse an existing field so allocate a new one
            bigger.update(biggerIdx, biggerVal.copy(dataType = merged))
          }
        }
        biggerIdx += 1
        smallerIdx += 1
      } else if (comp > 0) {
        bigger.insert(biggerIdx, smallerVal)
        // bump both indexes, the bigger struct has grown
        biggerIdx += 1
        smallerIdx += 1
      } else { // comp < 0
        // advance to the next field on the bigger struct
        // nothing else to do here
        biggerIdx += 1
      }
    }

@NathanHowell, I played around with your code from https://github.com/apache/spark/pull/12750#issuecomment-215607825 and it was a bit slower than mine because it turns out that comparing StructTypes for equality is really expensive and the benefits of avoiding object allocation were outweighed by the additional equality checks.
There are a couple of minor tricks which can improve the performance of your approach, though. In
if (biggerVal.dataType != smallerVal.dataType) {
          val merged = compatibleType(biggerVal.dataType, smallerVal.dataType)
          // test to see if the merged type is equivalent to one of the existing
          // StructField instances, reuse will reduce GC pressure
          if (smallerVal.dataType == merged) {
            bigger.update(biggerIdx, smallerVal)
          } else if (biggerVal.dataType == merged) {
            // do nothing, the bigger struct already has the correct field
          } else {
            // we can't reuse an existing field so allocate a new one
            bigger.update(biggerIdx, biggerVal.copy(dataType = merged))
          }
[...]`

note that  compatibleType's first step is to compare its inputs for equality, so if the two input types are equal then one of those inputs will be re-used as the value of merged. Thus, I think you can remove some checks by rewriting this as
          val merged = compatibleType(biggerVal.dataType, smallerVal.dataType)
          if (smallerVal.dataType eq merged) {
            bigger.update(biggerIdx, smallerVal)
          } else if (biggerVal.dataType eq merged) {
            // do nothing, the bigger struct already has the correct field
          } else {
            // we can't reuse an existing field so allocate a new one
            bigger.update(biggerIdx, biggerVal.copy(dataType = merged))
          }

After a bit more time in a profiler, I was able to make this 2x faster than my previous best time.
 Imagine the schema being the union of a huge number of smaller schemas, so it's pretty big and sparse.
My benchmark harness reads in the raw JSON, caches it, unions it together a bunch of times to inflate the benchmark runtime (to avoid small constant factor noise), then coalesces it to a single partition to avoid task-launch overheads (and so we measure single-core performance):
val lines = spark.read.text("...").rdd
lines.cache.count()
val coalescedLines =  (lines ++ lines ++ lines ++ lines ++ lines).coalesce(1)

for (i <- 1 to 10) {
  val startTime = System.currentTimeMillis
  spark.read.json(coalescedLines)
  println(s"Took ${System.currentTimeMillis - startTime}")
}

Before (f5da592fc63b8d3bc09d49c196d6c5d98cd2a013), this took around 88 seconds.
After (6bf5ee634ee7e509a88e64236f18e3c6e7a07aa2), this takes about 14.5 seconds.
Coupled with the changes in #12741, a change that led to a massive speedup in the reduce / treeAggregate step, this should be a huge speedup vs 1.6.x.

Test build #57421 has started for PR 12750 at commit 6bf5ee6.

Test build #57425 has started for PR 12750 at commit 48be267.

Test build #57425 has finished for PR 12750 at commit 48be267.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57425/
Test FAILed.

Jenkins, retest this please.

Test build #57429 has started for PR 12750 at commit 48be267.

Test build #57421 has finished for PR 12750 at commit 6bf5ee6.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57421/
Test FAILed.

Test build #57429 has finished for PR 12750 at commit 48be267.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57429/
Test FAILed.

Merged build finished. Test FAILed.

Jenkins, retest this please.

Test build #57433 has started for PR 12750 at commit 48be267.

Test build #57433 has finished for PR 12750 at commit 48be267.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57433/
Test FAILed.

Merged build finished. Test FAILed.

It looks like the failing test recently became flaky in the master branch (https://spark-tests.appspot.com/tests/org.apache.spark.mllib.stat.JavaStatisticsSuite/testCorr), so I'll come back and retest this after it's ignored / fixed there.

Jenkins, retest this please.

Test build #57461 has started for PR 12750 at commit 48be267.

Test build #57461 has finished for PR 12750 at commit 48be267.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57461/
Test PASSed.

;retest

Jenkins, retest this please

Test build #57557 has started for PR 12750 at commit 48be267.

Test build #57557 has finished for PR 12750 at commit 48be267.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57557/
Test PASSed.

Merged build finished. Test PASSed.

LGTM, just one minor comment.

Test build #58151 has started for PR 12750 at commit 8c7f7ad.

Test build #58151 has finished for PR 12750 at commit 8c7f7ad.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58151/
Test PASSed.

Merging this into master and 2.0, thanks!


Only import Unpack from typing on 3.11

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

Updated to last

synced with d5ecdc26605713ca6a83dd11daee84fe44f6dd14


New version: Google.Chrome.Canary version 118.0.5981.0

Pull request has been created with WinGet Automation :rocket:
Microsoft Reviewers: Open in CodeFlow

Reset PR


Small refactor and a bit clean up

The logic of ==(other) method was a bit confusing so I tried to clean it up to make it easy to read. I also moved new_from_response class method of Data into class << self for consistency.
Let me know if there is anything I can change and make it better.

Thank you @khezam for your contribution ❤
I might agree in principle on the == changes, but I'm concerned about unexpected changes for all existing clients.
This PR also lacks testing, but don't worry about test/spec live just yet, at this point they are expected to fail, we'll run them if we decide to proceed.
cc @snatchblock, what do you think of these changes?

My pleasure @ecoologic  :)
I see your concerns about unexpected changes for all existing clients and the lack of testing. I was/am expecting the existing tests would catch any unexpected behaviors since this PR only cleans up the current logic of ==.
I'm happy to add a test coverage if needed.
Let me know :)

Hey @ecoologic, any update on this PR?

Hi @snatchblock,
I definitely can add test cases, but this PR does not need test cases because it does not add/remove from the existing logic, it's only rearranging the existing logic. The test cases already exist here, and if I add test cases, they'll be exactly the same.
The two methods below are exactly the same but the only difference is that the top one is easy to read.
def ==(other)
    return false unless other

    return true if other.object_id == object_id

    return other.id && (other.id == id) if other.is_a?(Data)

    return id == other if other.is_a?(Integer)

    warn "Trying to compare #{other.class} to a Resource
      from #{caller.first}"
 end

  def ==(other)
      return true if other.object_id == object_id

      if other && !(other.is_a?(Data) || other.is_a?(Integer))
        warn "Trying to compare #{other.class} to a Resource from #{caller.first}"
      end

     if other.is_a?(Data)
        other.id && other.id == id
     elsif other.is_a?(Integer)
        id == other
     else
        false
     end
  end

Let me know if I'm missing anything and I'm happy to add test cases accordingly.


fix: fix running sail with sudo command

Goal
This PR will fix the problem mentioned in this issue#81
Problem
What caused to this problem arise?
This problem will occur if you call sail command with sudo or use docker in rootless mode.
Description
When a user uses sudo to call the sail command, these variables WWWGROUP and WWWUSER will set to the UID of the root user on their system (in most cases it should be 0). now if the user has its own user on the system with a different UID, the volume syncing between the host and the container will be set to the wrong number.
Example:
Imagine my user id is 1000, when I run the sail with sudo (and if I didn't set WWWGROUP and WWWUSER to my user id in the environment variable or docker-compose.yml), then sail's user id will set to 0 on the container. (actually it won't happen, cause the container already has the root user with the id of 0, if you check the sail's id on the container it will remain on the default value 1337).
but the actual owner of the files in the container is the one who has the id of 1000 (cause there is no user with this id on the container if you log the owner it will show the raw number that is 1000). in this situation when Laravel wants to write to a file on this directory it has no sufficient permission. and the errors about permission will throw.
I find out that there is a workaround to get the actual user when someone calls the sail with sudo. with this solution, everything will works as expected.
Note: besides using sudo there is a problem with the rootless mode of docker. in the rootless mode, docker will map all the permission to the root user in the container, and I couldn't find a workaround for this. then I suggest that you mention in the document that people consider not using docker in rootless mode if they want to use Laravel sail.

Don't use Sail in that way.

Then can you please add a note about giving root access to the docker if anyone wants to use Sail?
https://docs.docker.com/engine/install/linux-postinstall/

Just to clarify.
To work with Sail, Is better to run Docker as a root?
In other words, Is better not to use the rootless mode of docker?


fix(awsim_labs_vehicle_launch): update launch file

Description
Due to changes in the raw_vehicle_command_converter node, it became necessary to update the launch file. Necessary updates have been made.
Tests performed

Not applicable.
Effects on system behavior

Not applicable.
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.

[ ] I've confirmed the contribution guidelines.
[ ] The PR follows the pull request guidelines.

In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.

[ ] The PR follows the pull request guidelines.

Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.

[ ] There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

FYI: @mozhoku @Aysenayilmaz @xmfcx


bfloat16 Support

Anyway to use the model with bfloat16 data type?

Most RTNeural layers are agnostic to the input data type. Although they're mostly designed to work with floating-point data types, it's also 100% possible to use most RTNeural layers with types like int or std::complex<T> (although an RTNeural::TanhActivationT<int> might not be all that useful).
I haven't tried using RTNeural yet with a bfloat16 data type, but I imagine it should work, provided that the backend being used with RTNeural also supports bfloat16. I don't believe XSIMD has bfloat16 support. Eigen supports bfloat16 as of version 3.4, so that should work in RTNeural (RTNeural uses Eigen version 3.4.90). If you're using the STL backend, C++23 is supposed to have support for bfloat16, but I'm not sure what compilers support that feature at the moment.
We should probably add some tests to the RTNeural test suite, to explicitly make sure that bfloat16 is working with the available layers, and see if there are any potential issues there. I should have some time to work on that next week. In the meantime, if you already have a use-case for bfloat16, I'd be curious to hear if you run into issues (or if it "just works")!

Tried it doesn't seem to work with std:: bfloat16_t. It needs some of the parts to be statically casted, which solved only some parts of the issue. That would be really nice if you could have a quick look at it.
So the answer is, it doesn't work.

Thanks for the update!
Would it be possible to share a compiler log with any errors messages that may have resulted.
It would also be nice to know the compiler and RTNeural backend that you were using in your test.
If you already have a public repo or something similar where you've been doing your testing, that would be very useful as well!

Okay, I finally got around to testing this properly. At the moment, the only compiler I'm aware of that supports std::float16_t and std::bfloat16_t is GCC-13. With that specific compiler, I was able to add a few tests to RTNeural's test suite to test for compatibility with those floating-point types. The bottom line is that the tests work, however, there are a few disclaimers:

Support for float16_t and bfloat16_t only works with the Eigen and STL backends. I don't believe that XSIMD support half-precision floating point types, but I would imagine they might in the future.
In order for the tests to pass, I had to make make the bounds for acceptable error a bit larger. I think this is to be expected with lower-precision floating point types, but I haven't rigorously confirmed that the accuracy is as good a it can be... there might be some things that we can do in RTNeural to make the accuracy a bit better.
The tests that I've set up cover a good range of RTNeural's supported layers, but not all of them. If you run into problems using one or two specific layers with half-precision floating point types, then we can investigate those a bit more closely.
I haven't yet looked at the RTNeural's performance when using these half-precision floating point types. I wouldn't be surprised if it's not very good, depending on the backend/target platform, but that's something we can continue to investigate.
As mentioned above, using float16_t/bfloat16_t requires C++23 and GCC-13. As other compilers add support we can enable tests on those platforms as well.


Fix shape drawer#filled rectangle(rectangle rect)

Typo resulted in ShapeDrawer#filledRectangle(Rectangle rect) creating an outline instead of a filled shape. This change resolves the issue.

Thanks for the fix!


Added ThresholdTiler

Description
Added a new tiler that, given a scorer and a threshold, extracts all tiles whose score is above the threshold.
There may be instances when the user does not want the top n tiles from a slide, but rather every tile with a score over a specified threshold.
In this instance, the user does not know beforehand how many tiles must be extracted.

Types of Changes

[ ] Core
[ ] Bugfix
[x] New feature
[ ] Enhancement/optimization
[ ] Documentation

Issues Fixed or Closed by This PR
Checklist

[x] My code follows the code style of this project.
[x] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes.
[x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Hi Erich, thank you for your contribution!  I think it could be more intuitive to select a threshold when the scores are scaled between 0 and 1 rather than on raw scores, what do you think?

Hi Erich, thank you for your contribution!  I think it could be more intuitive to select a threshold when the scores are scaled between 0 and 1 rather than on raw scores, what do you think?

Hi 👋,
just checking @erich-r do you have any news on this PR?

Hi wave, just checking @erich-r do you have any news on this PR?

Hi!

I wrote the integration tests
Now the threshold is being compared with the normalized scores (as suggested by @nicolebussola )
Next I'm going to apply @alessiamarcolini suggestions (as soon as possible)

Hi wave, just checking @erich-r do you have any news on this PR?

Hi!

I wrote the integration tests
Now the threshold is being compared with the normalized scores (as suggested by @nicolebussola )
Next I'm going to apply @alessiamarcolini suggestions (as soon as possible)

awesome thanks a lot

hey @erich-r do you have any updates about this PR?


[vm/ffi] Expose dart_api.h and dart_native_api.h via dart:ffi?

We could expose some symbols from dart_native_api.h and dart_api.h via dart:ffi.
This would solve two problems:

The symbols not being exported by various embedders https://github.com/flutter/flutter/issues/46887.
Static linking issues on Windows #40579.

This would change the usage of dart_api.h and dart_native_api.h to manual dynamic linking passing back function pointers.
Static linking (current situation)
#include "include/dart_api.h"
#include "include/dart_native_api.h"

void SomeCode(){
  const bool result = Dart_PostCObject(send_port, &dart_object);
}

Manual dynamic linking (proposal)
// For the typedefs, but don't use the function pointers.
#include "include/dart_api.h"
#include "include/dart_native_api.h"

typedef bool (*Dart_PostCObjectType)(Dart_Port port_id, Dart_CObject* message);
Dart_PostCObjectType Dart_PostCObject_;

DART_EXPORT void RegisterDart_PostCObject(
    Dart_PostCObjectType function_pointer) {
  Dart_PostCObject_ = function_pointer;
}

void someCode(Dart_Port send_port, Dart_CObject dart_object) {
  const bool result = Dart_PostCObject_(send_port, &dart_object);
}

// dart:ffi
external Pointer<NativeFunction<...>> nativeApiDart_PostCObjectAddess();

// user code
final registerDart_PostCObject = dl.lookupFunction<
        Void Function(Pointer<NativeFunction<...>> functionPointer),
        void Function(Pointer<NativeFunction<...>> functionPointer)>(
    'RegisterDart_PostCObject');

registerDart_PostCObject(nativeApiDart_PostCObjectAddess());

cc @mkustermann @mraleph @derolf

Exposing those into Dart is also an option I thought about.
I could then pass them down to my native library.
That way also the native-finalizer topic can be solved easily.


Need of Realm for Normalize Cache

SQLlite is too slow for queries.
It would be such as great HTTP request but weak in Local data Needs.
A needed Features for Android Developers

Hi! Thanks for the suggestion! I'm curious what led you to the conclusion that SQLite was too slow. Do you have any numbers you can share ?
If needed, a realm normalized cache could be done by implementing the NormalizedCache API like it is done for the sqlite one.

I would also be against this to make it part of the library. Also because SQL operations are done in background thread, this should not be a problem. Also as @martinbonnin mention, it can be potentially implemented by users if it is really needed


test: Ensure we can run both ESM and CJS

Previously, the E2E test only tested a CJS app. This test ensures compatibility with ESM apps.

Can you take care of the linting issue, please?

Can you take care of the linting issue, please?

Done :+1:


reject packets with empty payloads

Sending packets with no frames is forbidden.

Rebased due to conflicts with the other PR.


vscode debug launch configuration

I'm not sure if this is useful for the project but I use vscode for debugging test cases

Coverage remained the same at 100.0% when pulling 8df293e5ff743d052a412f677c2047aca2ad449f on esetnik:feature/vscode-debug into 52693db875473a6321c01c6cd54b6f80cd2d2e38 on slimphp:master.

This isn’t something we want to include in this package. Thanks though.


Configuring Multiple Services with Credentials fails

If multiple services are configured, the catalog compilation fails due to the "carbon.dll" resource being declared twice. Example:
 windows_services::credentials { 'service A':
        username          => $service_user_name,
        password          => $service_password,
        servicename       => "service-a",
        delayed           => false,
        notify            => Service['service-a'],
}

 windows_services::credentials { 'service B':
        username          => $service_user_name,
        password          => $service_password,
        servicename       => "service-b",
        delayed           => false,
        notify            => Service['service-b'],
}

I suggest moving the File resource for the carbon dll into a separate class, which can be included for each service that is being configured:
class windows_services::carbon(
    $carbondll   = "C:\\Carbon.dll",
)
{
    file{"${carbondll}":
        source => "puppet:///modules/windows_services/Carbon.dll",
        source_permissions => ignore,
    }
}

Credentials.pp can then require the class and force ordering via chaining without causing a failure.

I'm noticing this issue as well, and sine I am new to Puppet I am having difficulty figuring out how Areson's proposed edit fits into the overall credentials.pp file.
Areson, any chance you could provide more details (or attach a file)?

Puppet doesn't let you include a resource for the same file more than once. The Credentials.pp file current includes the carbon dll as this:
  file{"${carbondll}":
    source => "puppet:///modules/windows_services/Carbon.dll",
    source_permissions => ignore,
  }

Which means that once you do your second call to windows_services::credentials it fails because that file gets added a second time. The way to bypass this is to put the file resource in its own class, and then require that without parameters. This effectively substitutes the above file resource in the Credentials.pp file with this:
require windows_services::carbon
Puppet can resolve this in the graph and it allows you do declare credentials for more than one service.

Got it.  In retrospect your original post had all the detail one should really need, but your second post made it click for me.  I can confirm that putting your proposed changes in place makes the module work perfectly fine when referenced more than once.  I appreciate you taking the time to respond and explain things a bit more.


Implement Correction formula for PM2.5

This applies to all models (ONE and Open Air).
We decided on (a rather complex) compensation formula for PM2.5. This will also be implemented on the firmware (only to show on the display) but the same formula should be applied in HA.
Formula and background information can be seen here:
https://github.com/airgradienthq/arduino/issues/182

Since the device already calculates this, would it make sense to have the device return this instead? This also allows for change in the future without a need for change on the integration side. This would also allow other integration platforms like the openhab to use the same logic without code change.

The algorithms might change in the future and having it on the receiving end makes it easier to deal with different firmware versions and ensures that all devices are treated with the same algorithm.

But that also means it's harder to use a different algorithm when using a different sensor type on a different device in the future.
Not saying I wouldn't do the change, but I'm just pushing back as I believe it should exist in the device firmware instead of the consumer. I mean if you are fine with the consequences that can occur (differences when using new firmware, old HA or new HA, old firmware), I am also fine with it.

We had originally the same idea and implemented the temperature correction on the firmware but then our whole science team got into issues because it wasn't clear anymore which monitors delivered raw data and which not etc etc. so they couldn't do their analytics anymore. Then we rolled back that change.
Additionally for some of the pollutants like PM there might be several different algorithms in future. So having the raw data coming in gives a lot more possibilities.
So on first thought yes it makes sense doing it on the fw side but then we realized it's actually a bad idea and has a lot of unintended consequences.

Would adding pm25Raw as an attribute help in that use case? Just like with the tvoc and Nox?

Then how do you call the non raw one? You would need to keep track on which firmware version has which algorithm and send that information together with the device or have some mapping ...
We really spend a lot of time thinking this through.
There is no ideal solution but we made a decision some weeks ago and implemented that approach already on our dashboard. So we should keep the same approach for other platforms otherwise things will get really messy.
The approach that we decided on is that we transfer raw data and do the conversions on the receiving end. I know it's not 100% ideal but we got into real troubles with the other one.

{
    "pm25": 12,
    "pm25Raw": 13,
    ...
}

It could look something like this. This way you can change the algorithm of the pm25 without a change in any consumer.
But hey, you're the experts and no doubt you thought this through. I'm just trying to challenge the requirement :).

@joostlek I got this completely wrong. I will close this issue and make a new one.


[BUG] can't collect logs but the log_collector status is OK

Agent Environment
version: 7.34.0,  deployed on k8s with DeamanSet way
with "datadog-agent status" command, I can see the Status is OK, but it seems that the log was not be  sent to server
- Type: file
  Identifier: 991668ee6cc5644e3736470d94e290fcb80a53dedcc42fcdd18a3828271bde25
  Path: /var/log/pods/online_solar-account-84c5d47d96-qbgvp_695989bf-e910-4ae7-b025-6aadcea90ade/http-server/*.log
  Status: OK
    1 files tailed out of 1 files matching
  Inputs:
    /var/log/pods/online_solar-account-84c5d47d96-qbgvp_695989bf-e910-4ae7-b025-6aadcea90ade/http-server/0.log
  BytesRead: 1.432902e+06
  Average Latency (ms): 0
  24h Average Latency (ms): 0
  Peak Latency (ms): 0
  24h Peak Latency (ms): 0

Describe what you expected:
Is there any possible problem would cause this case?

Hi @yutingcaiyt, thanks for using Datadog.
This issue queue is primarily intended for tracking features, bugs and work items associated with the integrations-core open source project. For your request, please contact our support and send them your logs.
Thanks.


Configuration.defaultMode is not honoured in 2.0.0-alpha1

I was just trying out the 2.0.0-alpha1 today and I suspect Configuration.defaultMode is not honoured.
Here is is some stripped down code from a Spock test
@Shared ProxyConfiguration configuration = ProxyConfiguration.builder().
        defaultMode( TapeMode.READ_WRITE   ).
        build()

@Betamax(tape='foo')
def "My test"() {
}

That always fails with a tape is read-only error.

I speak under correction, but I think this is the issue.
In the Betamax annotation, the default for mode is READ_ONLY.
hen in the RecorderRule, the annotation value is assigned and passed through to the start(tapeName, tapeMode, matchRule).
This ends up calling a Recorder.start method, which eventually ends up execyting the line tape.setMode(mode.or(configuration.getDefaultMode())). As the mode is already set mode.or will never be called.

I'll look at this over the weekend. I wasn't surprised that this may be an issue. Thanks for letting us know!

I suspect this is a problem with the match rules too. The global and recorder configurations will never kick  in, as you always(?) have a Betamax annotation. I'm not sure that any of the annotation's fields should have default values.

@pledbrook @ysb33r The many different ways to set the tapeMode is a problem anyway. There should be a running configuration (which almost always is READ_ONLY), and then there is a writing test configuration, which probably should be overwritten via java properties and not checked into git. That way, a developer is free to write, but CI will enforce that the proper tape is checked in. Thoughts?

In the mean time, I'll see if I can fix this with something quick and put it into the SNAPSHOT.

I like the idea, but I think this can be simplified to:

Default behaviour is READ_ONLY
The other modes remain configurable via existing mechanisms (ProxyConfiguration and @Betamax)
A betamax.allow.write or similar system property automatically overrides READ_ONLY/SEQUENTIAL to the corresponding write versions. It ignores any tests that are already marked as write.

The only difference from the existing behaviour is the last step.

@pledbrook I think we're generally in alignment. I just have to come up with an optional scheme that makes me happy, which may be the real challenge here since I prefer Scala over Java.

To paraphrase @pledbrook

If nothing is set the READ_ONLY
If a default is set via ProxyConfiguration then that becomes the default
If a value is set via @betamax, then that would override any of the previous two
System property beats all.

I'm good with that.

I prefer Scala over Java

I think most JVM people nowadays prefers another JVM langiage over Java :-)  Anyhow that is besides the point. I think if you need to find something that also works for Scala that cal also slot into the above list somewhere then do that.

Moving this into Trello: https://trello.com/c/qyTUkX75.


Bug: QA Fix Begin/End Date/Hour/Minute Validation

Context
QA Fix Begin/End Date/Hour/Minute Validation is not working properly.
There are a number of QA test types where the validation of Begin and End Date and Hour are not working properly.  For instance, End Date might be optional, but if you provide one then you also have to provide End Hour.  Or if you provide End Hour, then End Date cannot be null.
Note: All of the test types below have Begin/End Date/Hour/Minute Validation. Ensure the validation is working properly for each test type
Appendix E Correlation Test Run
Cycle Time Injection
QA Certification Event
Rata Run
Test Qualification
Test Summary
Unit Default Test
Steps to Reproduce

Log into ECMPS
Navigate to QA module and check out a config
Select a type that contains Begin/End Date/Hour/Minute Validation
Select view/edit
Edit the Begin/End Date/Hour/Minute

The validation should work as expected.  For instance, End Date might be optional, but if you provide one then you also have to provide End Hour.  Or if you provide End Hour, then End Date cannot be null.
Testing
Tester: @MaeganWood64

Please Note:  For TestSummary, refer to the Check Specifications for the validation requirements as they are different than for most other record types.  For instance, there is no requirement that Begin Hour is provided when there is a Begin Date, and vice-versa.

[Uploading... image.png]


tuw_msgs: 0.0.6-0 in 'melodic/distribution.yaml' [bloom]

Increasing version of package(s) in repository tuw_msgs to 0.0.6-0:

upstream repository: https://github.com/tuw-robotics/tuw_msgs.git
release repository: https://github.com/tuw-robotics/tuw_msgs-release.git
distro file: melodic/distribution.yaml
bloom version: 0.6.4
previous version for package: null

tuw_geometry_msgs

No changes

This conflicts with #17728. Please close obsolete PRs in the future to avoid this problem. This patch needs to be rebased now.

I'm going to close this as it's out of date an unmergeable..
@maxbader Could you resolve the conflict on this? If you resolve the conflict we can reopen it if you comment back here. Otherwise you can use bloom-release with the -p option to regenerate the pull request only.

Yes I am working on int

Thanks to tfoote, sorry for the inconvenience


Add more checks to emitc::CallOp verifier

This adds the following checks to the emitC::CallOp verifier:

args: arguments need a type
template_args: arguments need a type, forbid float literals, forbid dense elements attributes

I messed up during approving changes and re-requested reviews, but @simon-camp as well as I actually approved this.


fix cookie issues, add unit and e2e tests for cookies api

There are some issues with cookie deletion unresolved in this issue: https://github.com/sveltejs/kit/issues/6609
I believe the issue was that we were not setting the path on delete (and maybe on set.) Some tests are needed to confirm.
Marking this as draft until then.

[x] unit tests
[x] e2e tests (probably good to have more eventually, fix and unskip the flaky one)
[x] expose cookie serialize options from the SvelteKit cookies API (this should be a separate typescript issue) this is fixed by directly annotating the get, set and delete with JSDoc params rather than relying on the imported type of cookies
[x] document api changes, if any (e.g. setting path to other than '/') See this comment for suggested copy.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

[x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
[x] This message body should clearly illustrate what problems it solves.
[x] Ideally, include a test that fails without this PR but passes with it.

Tests

[x] Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

[x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

This is as far as I can take it, for now. I'd liked to have written more Playwright tests, but wasn't able to figure out fancier stuff, like testing enhanced forms. So the unresolved issue in #6609 I was only able to check by running the repro against this pr. But the perfect is the enemy of the good, and I'm confident that this is on the right track.
Net: The main issue folks were having stemmed from Kit not setting path and sameSite by default, for both set and delete. See also #6650. The defaults in this pr are exactly what I had been using to set/delete before the API, with no problems, so I'm pretty confident they are the right ones.

It looks to me like you added the same tests to client.test.js and server.test.js. If you want to run a test on both the client and server you can add it to test.js in the same directory

I just glanced quickly, but the Playwright tests look to me like they could cause flakiness. They run in parallel and are checking the same cookie values

Thanks @cdcarson — re the parallel-tests-causing-flakiness thing @benmccann referred to, you can set a group of tests to run serially like so:
https://github.com/sveltejs/kit/blob/e20425fefcfe570dc678d42f61d7e13483672fc0/packages/kit/test/apps/basics/test/client.test.js#L648

I moved the needlessly duplicate tests out of server and client into test.js.
Side note:

[The tests] run in parallel and are checking the same cookie values

I'm a playwright noob. Does this mean each test does not have its own browser?

Limit playwright tests snippet:
npx playwright test -g "Cookies API"

Ignore my previous comment about the tests running in parallel. It seems each test does get its own browser context: https://playwright.dev/docs/browser-contexts

I'm happy that this now fixes the issues we know about, knock on wood.
Remaining todos, not necessarily blocking.

Decide whether to keep encode and decode as options, or just go with the cookie library defaults (de/encodeURIComponent.) Currently we are keeping them. The API surface is only minimally larger, since they are optional and most folks will ignore them. On the other hand, the advantages of disabling bespoke encode and decode are (1) we would only have to read the cookies once and (2) there's very little case for having a bespoke solution for de/encoding 3 disallowed characters.
Documentation. IMO, basically the following. (Wasn't sure where this would go in the docs.)

Cookies
Unlike all other response headers, which you set with setHeaders, set-cookie headers should be set and deleted with cookies, which is available as part of RequestEvent.
// example tktk

The cookies API is  built on the cookie library, which you may already be familiar with. SvelteKit provides opinionated, sensible defaults for cookie serialization:
{
	httpOnly: true,
	secure: true,
	path: '/',
	sameSite: 'lax'
}

Also, implicitly, cookies are set to the session, so they will disappear after the user closes the browser. If you want a long-lasting cookie, set maxAge to the number of seconds you want it to last. Other defaults can be changed as necessary by passing your overrides to cookie.set and cookie.delete.
// example TKTK, including maxAge

Important: If you do use your own values for httpOnly, Secure, SameSite or path to set a cookie, you should use the same values to delete it. Some browsers are picky about this.
Safari and dev mode: Safari does not send Secure cookies to localhost over http://. To get around this... (TKTK not sure how far down the rabbit hole we want to go here.)

I'm having issues setting cookies when they're being passed between server to server requests. Also when setting them in the load function (in +layout.server.ts). My code was working fine before switching to the new cookies API. Would you happen to know if this might fix those issues as well?
Thanks...

@DoisKoh Probably. There are some bugs in the current implementation that this PR fixes.
@benmccann  Do I have to take care of the failing mac tests in the CI? These pass on my mac, and I'm not sure what's going on. Perfectly willing to try to figure it out, but want to check if this is something that happens often, and if there's a trick.  Thanks.

@benmccann Ignore my last qusetion about mac ci tests. I figured it out.

Thank you @cdcarson, this is awesome. I tried to make some changes locally and push them but for some reason it's not letting me, so I opened a new PR with those changes — we can either continue working in #6811 or you could merge that branch into this one.
The only real question I had was around the path default — it feels like this might be an opinion too far. Is it necessary?

The only real question I had was around the path default — it feels like this might be an opinion too far. Is it necessary?

@Rich-Harris Pretty sure it is necessary. See https://github.com/sveltejs/kit/issues/6650. Basically a cookie set in a request to /sign-in will have that path. A subsequent request to /profile won't send that cookie.
The only issue is if the site base path is /foo. We can probably address this in the docs.

Merged #6811, so I'll close this. Thanks @cdcarson!


Remove simpler render.yaml, along with other improvements

Main changes:

Remove the simpler setup render.yaml (which we'll submit to temporalio/temporal-render-simple), and make render.with_scaling.yaml the only render.yaml.
Make temporal-frontend a private server. Dependent services now rely on just the host property, and assume the frontend port is always 7233.
Make temporal-worker wait for temporal-history.
Make tctl work on all temporal services by setting TEMPORAL_CLI_ADDRESS in a bashrc file.
Remove the visibility DB, which isn't necessary when deploying with ElasticSearch. This involves overwriting config_template.yaml.

I'll open the PR to temporalio/temporal-render-simple after merging this, as that PR will link to this repo.

@davidmauskop Done!

🎉


Add support for Node v4

Hi,
Could you please add support for Node v4?
When I try to install Ghost through npm it currently generates lots of errors.
Thanks!

Closing as duplicate of #5821.
You can find an explanation why it doesn't work yet in this comment https://github.com/TryGhost/Ghost/issues/5821#issuecomment-138995279.


Generalize matrix function signatures

Description
Matrix functions can be generalized to work with Eigen expressions not just matrices. By propagating matrix expressions around we can avoid creating temporary matrices, improving performance. This could also make adding support for sparse matrices easier.
There was some discoussion about this on discourse. Also an abandoned PR #1309. Contrary to this PR I intend to split the this into smaller chunks:

[x] element-wise functions (ones that use apply_scalar_unary). Prim implementation of these can in many cases use Eigen implementations that can be faster than std.
[x] operator-like functions (add, subtract, multiply, divide, elt_multiply, elt_divide)
[x] size and view functions (col, row, sub_col, sub_row, head, tail, diagonal, transpose, cols, rows, num_elements, dims)
[ ] reminder of */fun
[x] */prob
[ ] allow functions to return expressions

Example
Instead of :
Eigen::MatrixXd f(const Eigen::MatrixXd& x)

We can do:
template<typename T>
auto f(const T& x)

Expected Output
More general and possibly faster code.
Current Version:
v3.0.0

+1 for this.  We also need to do this to allow support for sparse matrices.  @SteveBronder has been working on that along with perfect forwarding so we can reduce copying and exploit move semantics wherever possible.

We'd need to start coding a bit more defensively after implementing this, otherwise there's the risk of accidentally evaluating the expression multiple times. Take log_sum_exp for example:
template <typename T>
auto log_sum_exp(T& x) {
  if (x.size() == 0) {
    return -std::numeric_limits<double>::infinity();
  }

  const double max = x.maxCoeff();
  if (!std::isfinite(max)) {
    return max;
  }
  return max + std::log((x.array() - max).exp().sum());
}

If log_sum_exp is passed an expression, something like log_sum_exp(m1.array().exp().matrix() * m2), then that expression will get evaluated three times: x.size(), x.maxCoeff(), and the final return statement. So we'd need to start evaluating the expression at the beginning of the function, and probably also updating the documentation so that other/new developers know why

Right. I plan to do that in each function as I generalize its signature. At that time I can also make sure an expression is not used twice. Although I don't think just taking size of an expression evaluates it.
I also agree with the need to document this. Any idea where the doc should be put?

Absolutely.  Is the pattern to do something like:
Eigen::Matrix<T::Scalar, -1, 1> x_eval = x.eval()
and then use that in the rest of the function?
Evaluating .size() shouldn't need any evaluation. The expression template will already be evaluated and should at least know its size.  .maxCoeff() is another story.  So we'll have to dig in and see what's costly to evaluate and what's not.
It would be a service to devs to update the developer-facing doc.  I don't think this calls for any API-level doc changes other than of the acceptable input types for x here in log_sum_exp.

On Nov 29, 2019, at 6:05 PM, Andrew Johnson<EMAIL_ADDRESS>wrote:
We'd need to start coding a bit more defensively after implementing this, otherwise there's the risk of accidentally evaluating the expression multiple times. Take log_sum_exp for example:
template
auto log_sum_exp(T& x) {
if (x.size() == 0) {
return -std::numeric_limits::infinity();
}
const double max = x.maxCoeff();
if (!std::isfinite(max)) {
return max;
}
return max + std::log((x.array() - max).exp().sum());
}
If log_sum_exp is passed an expression, something like log_sum_exp(m1.array().exp().matrix() * m2), then that expression will get evaluated three times: x.size(), x.maxCoeff(), and the final return statement. So we'd need to start evaluating the expression at the beginning of the function, and probably also updating the documentation so that other/new developers know why
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.

:-)  I didn't see this before responding.  We're on the same page w.r.t. size().  I think by "evaluate" we mean evaluate operator()(i, j) for every (i, j) pair.  It always gets constructed.

On Nov 30, 2019, at 4:00 AM, Tadej Ciglarič<EMAIL_ADDRESS>wrote:
Right. I plan to do that in each function as I generalize its signature. At that time I can also make sure an expression is not used twice. Although I don't think just taking size of an expression evaluates it.
I also agree with the need to document this. Any idea where the doc should be put?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.

I have a little info on adding new docs to the stan site here
https://github.com/stan-dev/math/blob/develop/doxygen/howto_add_page.md
I wonder if for some functions we use so much info about the matrices it would be better to leave their template arguments as Matrix<T, R, C> just to force evaluation.

If a function needs to evaluate an argument and the template params for that arguments are known (not templated) we can just leave that argument as is  - const Matrix<T, R, C>&. However that does not work if we need template deduction at the same time. In that case I intend to use what Bob suggested.

If a function needs to evaluate an argument and the template params for that arguments are known (not templated) we can just leave that argument as is - const Matrix<T, R, C>&

You could also use the Ref<> functionality to maintain generality of input types, which (AFAIK) will take expressions as inputs and evaluate them into temporaries as needed. Using an example from Eigen's doc:
// read-only const argument:
void foo2(const Ref<const VectorXf>& x);

MatrixXf A;

foo2(A.row());              // Compilation error because A.row() is a 1xN object while foo2 is expecting a Nx1 object
foo2(A.row().transpose());  // The row is copied into a contiguous temporary
foo2(2*a);                  // The expression is evaluated into a temporary
foo2(A.col().segment(2,4)); // No temporary

Although some of the copies might be too expensive to be worth it

First, I thought Ref didn't work with slicing, segmenting, etc., but it seems to work OK.  Second, I thought it was type not size that determined passing a VectorXd, but the following compiles and runs.
What are the rules for executing into temporaries?  I wouldn't have thought any of those template expressions would cause an intermediate.
void foo2(const Eigen::Ref<const Eigen::VectorXd>& x) {
  std::cout << x << std::endl;
}

TEST(foo, bar) {
  Eigen::MatrixXd A(2, 2);
  A << 1, 2, 3, 4;
  foo2(A.row(0).transpose());

  Eigen::VectorXd a(2);
  a << 10, 100;
  foo2(2 * a);
  foo2(A.col(0).segment(0, 1));

  Eigen::MatrixXd B(2, 1);
  foo2(B);
}

This page says:

By default, a Ref can reference any dense vector expression of float having a contiguous memory layout. Likewise, a Ref can reference any column-major dense matrix expression of float whose column's elements are contiguously stored with the possibility to have a constant space in-between each column, i.e. the inner stride must be equal to 1, but the outer stride (or leading dimension) can be greater than the number of rows.
In the const case, if the input expression does not match the above requirement, then it is evaluated into a temporary before being passed to the function.

Thanks---that really clarifies what's going on.  I wish we'd figured this out a few years ago!

On Dec 4, 2019, at 2:24 AM, Tadej Ciglarič<EMAIL_ADDRESS>wrote:
This page says:
By default, a Ref can reference any dense vector expression of float having a contiguous memory layout. Likewise, a Ref can reference any column-major dense matrix expression of float whose column's elements are contiguously stored with the possibility to have a constant space in-between each column, i.e. the inner stride must be equal to 1, but the outer stride (or leading dimension) can be greater than the number of rows.
In the const case, if the input expression does not match the above requirement, then it is evaluated into a temporary before being passed to the function.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.

I am running out of ideas how to group remaining functions into PRs. So I will just go in alphabetical order.

I am running out of ideas how to group remaining functions into PRs. So I will just go in alphabetical order.

Can we make a list of the remaining ones as an issue? I think you put one up somewhere a while ago though I couldn't find it. Would be good so we could divvy up work if anyone else is interested. I started picking apart some of the stuff for read_corr/cov_L etc.

No, I never made such a list. If you have a list of all functions in Stan Math I am happy to check ones that I already completed.

I think Steve is reffering to the list of functions we made for the kernel generator.

Ah yeah nvm wrong one. Yeah I can look through and slam an issue up. We can use it as the issue for the remaining ones

I don't think we need another issue for this as we have this one. Feel free to either post here or edit top level comment once you make the list.

We are very close to having all functions accept expressions, so I am thinking about the details of returning expressions. Whenever a function returns an expression referencing a local variable it needs to use holder (see #1775).
When a function returns an expression referencing a function argument it gets a bit less clear if a holder is necessary. In majority of use cases (including all possible uses from stan language) everything should work just fine without holder. However in c++ a function can be used in a way that the returned expression is not evaluated before its rvalue argument(s) get deleted. For example this happens if the resulting expression is stored in an auto variable. Example (assuming holder is not used in softmax):
Eigen::MatrixXd a = ...
Eigen::MatrixXd b = ...
auto c = softmax(a+b); //after this statement sum expression is destructed and softmax expression is left with a dangling reference
Eigen::MatrixXd d = c+1; //now the softmax expression c is evaluated

So the question is, should we use holder for such cases (to make code safer) or not (avoiding overhead of creating holder and simplifying code? @SteveBronder @bbbales2 @andrjohns

My first reaction is holders cause it's less to worry about then.
How does this code compare to something written with Eigen? Like:
#include <Eigen/Core>
#include <iostream>

int main() {
  Eigen::MatrixXd a = Eigen::MatrixXd::Random(2, 2);
  Eigen::MatrixXd b = Eigen::MatrixXd::Random(2, 2);
  auto c = (a + b).array().abs().sqrt();

  std::cout << c + 1.0 << std::endl;

  return 0;
}

Is Eigen having to allocate holder things along the way?

This example would work with eigen. However if you replaced addition with a function that returns a plain matrix (not expression) it would have the same issue.

Also eigen does not use anything like holder. In functions in Eigen, where such dangers would happen they usually evaluate the result of the function instead of returning an expression.

Oh interesting. Adding an eval causes a memory problem here (had to turn on the sanitizer to get it). Weird: https://godbolt.org/z/M6sdGc
And what is the overhead of allocating a holder? Is it a malloc and a free, basically? Or is it something more?

Oh, I overlooked eval in your example. Yeah that is why it fails too.
About overhead: more or less just malloc and free.

Nah nah, I added the eval after you said the plain type would cause it to fail. Tricky!

should we use holder for such cases (to make code safer)

So using a holder in this case means our softmax would allocate a holder for its argument? Or where would the holder appear in the code?

Yes softmax would allocate it (if the argument is rvalue). You can look at implementation of value_of to see how holder is used.

I've been thinking about this some more. If we use holder for arguments, we need to do perfect forwarding on these arguments, as make_holder() needs to now whether it is dealing with lvalues or rvalues. But for these functions to actually know whether they have lvalue or rvalue wee need to forward arguments to them. So we need to use perfect forwarding in all functions that can call any function using holder. In other words in almost all functions.
On the other hand avoiding auto is not that hard, so I am leaning towards not using holder for arguments.

I've been thinking about this some more. If we use holder for arguments, we need to do perfect forwarding on these arguments, as make_holder() needs to now whether it is dealing with lvalues or rvalues.

My cargo culting was not all wrong! :laughing:
If we really wanted to go this route I think I could come up with a regex that searches over function signatures and replaces const (TemplateName)& with (TemplateName)&&. Though then will have to start dealing with C++'s memory semantics which adds a non-trivial layer of complexity. The overhead of that could probably be mitigated a bit by good documentation. Though then we would use make_holder() adding one more level of complexity. We'd need like a Kurzgesagt style youtube infographic video to explain all of that
Or alt, could we have a make_holder that takes in a type like make_holder<T_A>(A) and for plain types does nothing but for expressions does the alloc/free? My thought here is that when we see something like an Addition<Mat, Mat> come into an function it's almost always going to be an rvalue. Though then there's Qs about Block<> types and dealing with things like Block<Addition<Mat1, Mat2>>.
It feels like kind of a bummer to ban auto. What happens with something like softmax(softmax(a + b))? Would a+b's expr still get erased here?

I am still trying to avoid littering all the code with calls to std::forward and make_holder, which even ignoring the work to get there would make code less readable.
That second paragraph is more or less how Eigen expressions work. That works in most cases but fails as soon as it gets a rvalue plain type.

It feels like kind of a bummer to ban auto

It would not get completely banned. Just banned unless you know what you are doing. :D

What happens with something like softmax(softmax(a + b))? Would a+b's expr still get erased here?

No. Temporaries are destructed after everything in a line of code gets executed. If expression referencing temporaries is evaluated in the same line as these temporaries are created everything works fine.

It would not get completely banned.

Could we have our eval function handle this?
auto x = eval(...);

Like if you're using stan::math and you're assigning to an auto variable, eval. That's how I understand Eigen lol.
I guess that means the difference between us and Eigen then is that we can't have expressions as l-values and Eigen can?

Could we have our eval function handle this?

It can be used like this if you want.

I guess that means the difference between us and Eigen then is that we can't have expressions as l-values and Eigen can?

Nope. We are using Eigen expression anyway.

Well Eigen can do:
auto c = (a + b).array().sqrt(); // somehow eigen's sum survives
auto d = c + 1.0;

And we cannot do:
auto c = softmax(a + b); // our sum gets destructed
auto d = c + 1.0;

@t4c1 can we start removing the code like
    return ret_type(ret);

In functions that use reverse_pass_callback()?
https://github.com/stan-dev/math/blob/develop/stan/math/rev/fun/elt_multiply.hpp#L42
If it's safe to pass around arena matrices now I can also put up a lil PR in stanc3 that tests making stuff in the parameters block into arena matrices

Actually wrt stanc3 here's a lil brutal version that uses arena_t as the matrix type when the scalar type of the matrix is a var
https://github.com/stan-dev/stanc3/compare/master...SteveBronder:feature/arena-var-matrix
Would could take off the ret_type() yada from the stuff the uses reverse pass callback rn and get an eyeball on how much we save there. Though imo the bigger thing is going to be doing some form of arena_t<> with the data

can we start removing the code like

return ret_type(ret);
I'm still a bit nervous about this unless something has happened and we're talking about var<mat> and not mat<var>.
Check this code out:
Eigen::Matrix<stan::math::var, -1, -1> myfunc1() {
  Eigen::MatrixXd res_val(1, 1);
  res_val << 1.0;

  stan::arena_t<Eigen::Matrix<stan::math::var, -1, -1>> res = res_val;
  stan::math::reverse_pass_callback([res]() mutable {
      std::cout << "myfunc1" << std::endl;
      std::cout << res.val() << std::endl;
    });

  return res;
}

auto myfunc2() {
  Eigen::MatrixXd res_val(1, 1);
  res_val << 1.0;

  stan::arena_t<Eigen::Matrix<stan::math::var, -1, -1>> res = res_val;
  stan::math::reverse_pass_callback([res]() mutable {
      std::cout << "myfunc2" << std::endl;
      std::cout << res.val() << std::endl;
    });

  return res;
}

TEST(AgradRevMatrix, test_arena_output) {
  auto a = myfunc1();
  a(0) = 2.0;
  auto b = myfunc2();
  b(0) = 3.0;

  stan::math::grad();
}

In both cases the function sets the result value to 1, but this is the output:
myfunc2
3
myfunc1
1

The arena_t<Matrix<var, -1, -1>> is playing the same role as the vari ** s in the old varis, and if we don't make a copy of that on the output we're in danger of overwriting things.

Oof yeah now that I think about this, any assign() statement could cause this. The only thing I can think of to have it would work be to do the same tricky stuff I'm working on right now for var<mat> assign where we save the previous slice and re-apply it as a callback in the reverse pass

Well Eigen can do: And we cannot do:

The difference is that softmax needs to create a local matrix, which is than referenced by returned expression and rsqrt only needs to reference the given expression.

@t4c1 can we start removing the code like

Yeah, Ben is right. We can do it for var. For mat we need the copy. But mat will eventually be gone anyway.

Also I am assuming you are ok with not using holder for arguments.

The difference is that softmax needs to create a local matrix

Yeah and is that not true for all our functions?
I think what we'd write to document this is:

If a Stan expression is going to be used as an lvalue, it shouldn't depend on any rvalue temporaries. Practically this means that an expression saved as an lvalue should only use one function call, because the intermediates allocated in a sequence of function calls will get destructed.

Now that I write that it doesn't sound that clear, but that's what I'm thinking.
I feel like Eigen must have something special going on that we don't cause its expressions merge into bigger and bigger expressions that don't necessarily go away. The reason I'm making the direct comparison to Eigen is it would be cool if we could say "Stan expressions have the same limitations as Eigen expressions", which I don't think is the case cause our functions don't build giant expression types in the same way.

But mat will eventually be gone anyway.

var<mat> design doc needs an update. Assigning to the sub-matrix needs talked about and this seems like a big change of plans too.

Also I am assuming you are ok with not using holder for arguments.

Yeah I'm cool with this. It's the same with any oddity. It's fine for the language since we can hide it, and it needs doc'ed for Math so someone doing C++ doesn't get confused and mad.

Yeah and is that not true for all our functions?

Not for all ,but for many it is true. What I am trying to say is it is not that we are doing things things in different way in our functions - we are doing different things.

it would be cool if we could say "Stan expressions have the same limitations as Eigen expressions"

Actually we are better - we have less limitations! In cases such as softmax that need internal matrices, we will use holder, so that will not be an issue anymore. Eigen's way of doing it would be to simply give up on expressions and eval the result.
I guess softmax was not the best example to start this discussion with, as it will need holder anyway.

In cases such as softmax that need internal matrices, we will use holder, so that will not be an issue anymore
I guess softmax was not the best example to start this discussion with, as it will need holder anyway.

Oh okay so we are going to add some holders, just not holders everywhere. Yeah @ me when you put up this pull so I can stare at it a bit.

It will be up in 5 min :)


Typed Holes + Type Wildcards?

Haskell, Idris, Agda, PureScript and some other languages have a couple of different features that are useful for figuring out the types of functions or values:

Typed Holes: Being able to put a value that acts like a placeholder. If the type checker can figure out what the inferred type of that placeholder should be, it tells the user.

Partial Type Signatures, or Type Wildcards: Being able to put in a type that acts like a placeholder. If the type checker can infer from function implementations what the type should be, it tells the user.

For clarity, here's an example of each (in PureScript) respectively:

And the closest I've managed to get with Flow (same order):

In both of these cases, type-checking should "fail", to stop external tools thinking a dev leaving a type-debugging placeholder in the code is hunky-dory; it isn't, and should explode.
The typed-hole suggestion possibly breaks with Flow's current model (i.e. Flow being an overlay that can be removed at any time to result in JavaScript that just runs), but I'm considering it to be something that should never end up in finished real type-stripped code.
I use these a lot with my day-to-day in Haskell and PureScript, to figure out the types of things when I'm deep inside some expression, and I reach for them in similar cases when I go to write Flow. Any thoughts?

Seems like a dupe of a questin asked 3 hours prior to this one: https://github.com/facebook/flow/issues/2750

Haha, oh dear. I did have a look for issues, but seemed to miss that one, sorry. 💦
There's some overlap in the sense that it's talking about using inference to help the dev experience, yeah, but I don't really think it's a dupe. Happy to be corrected, though.

Yes, you are right. The linked issue seems to be concerned with the repl only while yours is more general. Type holes are not that old in Haskell too :)
BTW: I just opened flow's issues (haven't used flow at all) and saw two almost consecutive questions that seemed like they asked the same :)

Thanks for linking #2750, @gcnew. It's a different workflow (needs IDE integration, eg. for tooltips or a type-under-cursor shortcut), but as far as I can tell flow type-at-pos <file> <line> <col> can handle both the cases I was talking about:
Typed Holes equivalent (check type of var):

Type Wildcard equivalent (inferring return type of a function, in this case):

Not sure whether or not to leave this issue open. I've edited the first message; I hope there's enough of a summary to make a quick decision either way as to whether this is a useful suggestion or not.

Okay. The tooltips approach for works, but really only for inspecting already-compiling code. I haven't figured out a workaround[1] that works as well as type holes for the not-yet-compiling case.

Besides the one in the very first post, eg. var hole: void; for values; no idea what to do for functions.

type-at-pos in IDE should be able to give you the info. We don't intend to introduce other type holes.


Warlock of the Fiend Monster Fixes

Warlock of the Fiend Core changes:

On the ability Hellfire, the necrotic damage imports in the versatile field when it should be a second damage formula instead.
On the ability Hellfire, the saving throw imports flat when it should be derived from charisma
On the ability Fiendish Rebuke, the saving throw imports flat when it should be derived from charisma

Warlock of the Fiend Midi/auto effects changes:

Same as Core above
Same as Core above
Same as Core above
For Hellfire and Fiendish Rebuke, flag them as magical damage, magical effect, and half dam save(I never trust the parsing of descriptions specially when theres 2 damage formulas)
I automated the Dark One's Own Luck in Tpsoney's discord will link it here for you to grab, but understand theres currently a bug in midiqol, it does not like apostrophe's in item names so my copy deletes the apostrophe.  he may fix this before you ever read this.

Here is the link to Dark Ones Own Luck(without the apostrophe)(its tposney's discord)
https://discord.com/channels/915186263609454632/1112091379561152599/1112091379561152599

Fiendish Rebuke also is missing its 3/3 long rest

Fiendish Rebuke fixed in next release, Hellfire damage is fixed.
The saving throws on features is a separate issue. They aren't technically linked to a spell casting modifier, much like how Hellfire and Fiendish rebuke are technically not spell/magical effects. It's harder to determine what the save dc for these is based on (when parsing) so will leave this as a future project to add.

Closing this, and tracking flat saving throws in #337


[WIP] [AIRFLOW-1504] Log dataproc cluster name

Dear Airflow maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA

[x] My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

https://issues.apache.org/jira/browse/AIRFLOW-1504

Description

[x] Here are some details about my PR, including screenshots of any UI changes:
Logs the substituted cluster_name of the dataproc operator when it begins execution, making it easier to interpret errors mentioned in the Jira issue. The log is of the form "Creating cluster: <cluster-name>"

Tests

[x] My PR adds the following unit tests OR does not need testing for this extremely good reason:
test.contrib.operators.test_dataproc_operator.test_cluster_name_log_no_sub
test.contrib.operators.test_dataproc_operator.test_cluster_name_log_sub

Commits

[x] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

Subject is separated from body by a blank line
Subject is limited to 50 characters
Subject does not end with a period
Subject uses the imperative mood ("add", not "adding")
Body wraps at 72 characters
Body explains "what" and "why", not "how"

Codecov Report

Merging #2517 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2517      +/-   ##
==========================================
- Coverage    70.1%   70.09%   -0.01%
==========================================
  Files         150      150
  Lines       11545    11545
==========================================
- Hits         8094     8093       -1
- Misses       3451     3452       +1

Impacted Files
Coverage Δ

airflow/models.py
87.29% <0%> (-0.05%)
:arrow_down:

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 42cad60...32ab5a3. Read the comment docs.

+1 LGTM


database/sql: TestTxCannotCommitAfterRollback flake

The test introduced by this change golang.org/cl/216240 has been flaking (albeit rarely) on both dashboard builders and trybots.
greplogs reveals:
2020-04-22T16:45:05-141b11d/windows-amd64-2008
2020-04-20T20:22:51-7004be9/windows-amd64-2008

and I have a trybot failure for an unrelated change: https://storage.googleapis.com/go-build-log/b4a5cdf0/linux-amd64-race_056b784c.log
The failure, to repeat it here, is:
--- FAIL: TestTxCannotCommitAfterRollback (0.00s)
    sql_test.go:2802: sql: no rows in result set
FAIL
FAIL	database/sql	0.724s

CC @kardianos @odeke-em

Those two failures were already noted and fixed with https://go-review.googlesource.com/c/go/+/229478 .

Got it, my mistake was I only looked for open issues, and the base for my CL was a bit behind. Sorry for the noise.


Fix build on macOS with latest cc crate

cc v1.0.58 broke the macOS build by including the "-arch" flag in the
default set of compiler flags. Strip it out, like we do for iOS targets.
Closes #66.

👍
Thanks!


Specify the thread where callback handler is executed

Hi,
I have used APIKit and I want to say thank you.
I'm trying to use this library for command line application with Commandant and ReactiveCocoa.
I tried to use SignalProducer#single() in order to get result synchronously, but it doesn't get result.
Currently, callback handlers always are executed on main thread.
For command line application, it is good to specify the thread where callback handler are executed.
I tried to modify to specify the thread, it works for me.
Please use it as a reference.
Thank you.

Looks nice 👍
It would be good to extend this for NSOperationQueue and private queue of Session:
public enum CallbackQueue {
    case Main
    case SessionQueue
    case OperationQueue(NSOperationQueue)
    case DispatchQueue(dispatch_queue_t)
}

I'll work on another PR soon. Thank you for the nice suggestion ✨

Closing this in favor of #157.
@kumabook Could you confirm #157 works as expected in your case?

@ishkawa Thank a lot! I confirmed. It worked perfectly fine for me! And the implementation looks more swifty!


.Net Gemini Connector - Allow Version Selection and Base Url overriding.

The v1beta part of the URL is going to change and there may be times when multiple versions will be supported at the same time.

We should have:

A central place where the default API version is set
We should consider if we want to allow developers to change this. We do allow this in Azure OpenAI. It's a risky thing for a developer to do but that doesn't mean someone may have a good reason and we don't necessarily want to block them.

Basically when using AddGeminiChatCompletion we should be able to specify the target version in the service Settings, this will be adjusting the correct Uri of the client v1beta or v1, etc.
Not very sure about be able to override the endpoint as I don't see a potential mock or virtualization of GCP to be targetted (edge case).

I also think creating custom endpoint overload doesn't make sense.


Reorganize research data page

Currently connections can be listed twice. If connected, data import results are listed at the top. Then, all potential connections with possible actions (connect or redo import) are listed at the bottom.
Reorganize page to only have a section for each connection once (don't list connected sources on the bottom again). In other words, once a connection is made, present both resulting files and available actions for that connection together on the top.
The research data page is the one located here: https://www.openhumans.org/member/me/research-data/

thanks

:)

Both the default: https://www.openhumans.org/
And https://www.openhumans.org/research/
Look much improved! Thanks @madprime
Are you happy with it? Opportunity to close an issue?

@BenjaminHCCarr the 'research data page' refers specifically to this page:
https://www.openhumans.org/member/me/research-data/
there's still some work we'd like to do in cleaning that one up :)

@beaugunderson ah okay, yes, that is a little clunky. Just so I don't create another thread why is activities-list a div call # and not a regular URL:
works: https://www.openhumans.org/#activities-list
doesn't: https://www.openhumans.org/activities-list
the 404 is a little odd for that too:
<body id="page-activities-list" class="404">
I didn't think the class="404" could go in a  tag, but you're the expert, I thought it had to be a in proper
This odd activities-list page also make linking harder. Like my issue #567 example, the url for circles is:
<a href="https://www.openhumans.org/#activity-direct-sharing-2">
where I think it should be ( and what I tried first)
'https://www.openhumans.org/activities-list#activity-direct-sharing-2'
I haven't dealt a ton with node (and obviously you have), but is it not possible to root the dynamic page without the leading #?
it would make referential links much cleaner.

@BenjaminHCCarr
I added the anchor as an afterthought. The goal of #526 was to eliminate the "Activities" tab and simply have that list be the home page. But for the logged out user, there's some elements displayed before the activities list. So I added an anchor to let us have a link that jumps the user to the activities list (in case we wanted to reference it for a user that hasn't joined or isn't logged in).
The id and class for "body" are automatically generated in the functions here: https://github.com/PersonalGenomesOrg/open-humans/blob/master/open_humans/templatetags/utilities.py#L154
https://github.com/PersonalGenomesOrg/open-humans/blob/master/open_humans/templatetags/utilities.py#L167


[v2] Rewrite

If I have the time over the Christmas break I'd like to rewrite the core of PT a 3rd time. The front-end UI would largely be the same, but the front-end code would be completely refactored.
Some goals:

No callbacks, use async/await with promises. At the moment the codebase is a clown fiesta of callbacks, largely because I learnt JS while writing PT. If we swap to async/await some situations will be much easier to handle, like waiting for PTPlayer to subtly seek-over-time to get back inline with the host. This would also allow for new features to be added much more easily.
Vuex store broken down in to modules.
Improve PTPlayer compatibility to allow for direct play.
Navigation rewritten so that everything isn't embedded within /sync. Everything that requires the user to be authenticated, connected to a server and in a room should be handled within App.vue. All other routes should be handled within the router-view of App.vue.
Potentially rewrite the server. At the moment the server codebase isn't too bad, but I'd really like to have 0 callbacks. Currently I'm somewhat afraid of touching the server code as it works (and works well), and swapping to a promise-based codebase would help.

Just came across this project, browsing around a bit and I find this.....  A clown fiesta!!!!  That's the best thing I've heard all year...

DONEZO!