Fill in PR Parameters #21940

[ ] Have you signed the Contributor License Agreement?
[ ] Have you checked that there aren't other open pull requests for the same manifest update/change?
[ ] Have you validated your manifest locally with winget validate --manifest <path>?
[ ] Have you tested your manifest locally with winget install --manifest <path>?
[ ] Does your manifest conform to the 1.0 schema?

Note: <path> is the name of the directory containing the manifest you're submitting.

Fixes #21940

@OfficialEsco Please test this version of YamlCreate.ps1

This part does not work at all
Does this pull request resolve any issues?
Enter issue number. For example: 21983, 43509
[Y] Yes  [N] No (default is 'N'): 2
Warning: 1 uncommitted change

Also you don't actually need the $PSScriptRoot\..\.github\PULL_REQUEST_TEMPLATE.md part,
You can do
if ($Task -eq Yes) {
$PRTemplate += "`n[X] Task"
} else {
$PRTemplate += "`n[ ] Task"
}

or maybe you could use the PR Template as a Schema?
$PRTemplate = Get-Content -path $PSScriptRoot\..\.github\PULL_REQUEST_TEMPLATE.md
foreach ($Line in $PRTemplate {
  Prompt Line without [ ]
  If yes [X] insert else insert [ ]
}

This part does not work at all
Does this pull request resolve any issues?
Enter issue number. For example: 21983, 43509
[Y] Yes  [N] No (default is 'N'): 2
Warning: 1 uncommitted change

I have fixed this part.

Also you don't actually need the $PSScriptRoot\..\.github\PULL_REQUEST_TEMPLATE.md part,
You can do
if ($Task -eq Yes) {
$PRTemplate += "`n[X] Task"
} else {
$PRTemplate += "`n[ ] Task"
}

or maybe you could use the PR Template as a Schema?
$PRTemplate = Get-Content -path $PSScriptRoot\..\.github\PULL_REQUEST_TEMPLATE.md
foreach ($Line in $PRTemplate {
  Prompt Line without [ ]
  If yes [X] insert else insert [ ]
}

I first tried to take a variable and then use $PrBody = Get-Content -path $PSScriptRoot\..\.github\PULL_REQUEST_TEMPLATE.md, then replace it as you mentioned, but GitHub CLI does not take it line by line and what you will get is
- [ ] Have you signed the [Contributor License Agreement](https://cla.opensource.microsoft.com/microsoft/winget-pkgs)?- [ ] Have you checked that there aren't other open [pull requests](https://github.com/microsoft/winget-pkgs/pulls) for the same manifest update/change?- [ ] Have you validated your manifest locally with `winget validate --manifest <path>`? - [ ] Have you tested your manifest locally with `winget install --manifest <path>`?- [ ] Does your manifest conform to the [1.0 schema](https://github.com/microsoft/winget-cli/blob/master/doc/ManifestSpecv1.0.md)?   Note: `<path>` is the name of the directory containing the manifest you're submitting.  -----

I have fixed this part.

Yap less confusing now

I first tried to use $PrBody = Get-Content -path $PSScriptRoot...github\PULL_REQUEST_TEMPLATE.md, then replace it as you mentioned, but GitHub CLI does not take it line by line and what you will get is a string like this, that's why I need to write such a long function.

Took me waaay too long to test gh pr create 🤦♂ You're right, it actually needs a file.. thats dumb
What do you think of something like this? (Testing draft)
Function Enter-PR-Parameters {
    Clear-Variable PrBodyContentReply
    $PrBodyContent = Get-Content -Path "$env:USERPROFILE\Github\winget-pkgs\.github\PULL_REQUEST_TEMPLATE.md"

    ForEach ($_ in ($PrBodyContent | Where-Object { $_ -like '-*[ ]*' })) {
        if ($_ -like "*winget validate*") {
            $PrBodyContentReply += $_.Replace("[ ]","[X]"), "`n"
        } else {
            Write-Host
            Write-Host
            Write-Host -ForegroundColor 'White' $_.TrimStart("- [ ]")
            Write-Host -ForegroundColor 'White' -NoNewline "[Y] Yes  "
            Write-Host -ForegroundColor 'Yellow' -NoNewline "[N] No "
            Write-Host -NoNewline "(default is 'N'): "
            do {
                $keyInfo = [Console]::ReadKey($false)
            } until ($keyInfo.Key)
            if ($keyInfo.Key -eq 'Y') {
                $PrBodyContentReply += $_.Replace("[ ]","[X]"), "`n"
            } else {
                $PrBodyContentReply += $_, "`n"
            }
        }
    }

    Write-Host
    Write-Host
    Write-Host -ForegroundColor 'White' "Does this pull request resolve any issues?"
    Write-Host -ForegroundColor 'White' -NoNewline "[Y] Yes  "
    Write-Host -ForegroundColor 'Yellow' -NoNewline "[N] No "
    Write-Host -NoNewline "(default is 'N'): "
    do {
        $keyInfo = [Console]::ReadKey($false)
    } until ($keyInfo.Key)
    if ($keyInfo.Key -eq 'Y') {
        Write-Host
        Write-Host "Enter issue number. For example`: 21983, 43509"
        $ResolvedIssues = Read-Host -Prompt 'Resolved Issues' | TrimString
        Foreach($i in ($ResolvedIssues.Split(","))) {
            $PrBodyContentReply += "Resolves #$i`n"
        }
    } else {Write-Host}

    Write-Host
    $PrBodyContentReply = ($PrBodyContentReply.Trim() -ne '')
    $PrBodyContentReply

    #Set-Content -Path PrBodyFile -Value $PrBodyContentReply | Out-Null
    #gh pr create --body-file PrBodyFile -f
    #Remove-Item PrBodyFile
}

I have this issue:
Does this pull request resolve any issues?
[Y] Yes  [N] No (default is 'N'): y
Enter issue number. For example: 21983, 43509
Resolved Issues: 123, 1234

- [X] Have you signed the [Contributor License Agreement](https://cla.opensource.microsoft.com/microsoft/winget-pkgs)?
- [X] Have you checked that there aren't other open [pull requests](https://github.com/microsoft/winget-pkgs/pulls) for the same manifest update/change?

- [X] Does your manifest conform to the [1.0 schema](https://github.com/microsoft/winget-cli/blob/master/doc/ManifestSpecv1.0.md)?
Resolves #123
Resolves # 1234

For no space i have to do Resolved Issues: 123,1234
Dunno if its because i've messed with your part
Also i agree the Test-Manifest should not be there 😁
Also the script already runs winget validate, so i don't think there is a reason for it do do it again so we just check validate automatically .

Also i agree the Test-Manifest should not be there 😁

Fixed in commit 0daf9f0bfda3686ce3a653753d4ac83d46bfdd4c

For no space i have to do Resolved Issues: 123,1234

Fixed in commit 4f24c84993f925dcc73c838f42d734adf989e2a1... Now i think everything is perfect...

Also the script already runs winget validate, so i don't think there is a reason for it do do it again so we just check validate automatically .

You are right... I have removed that line in commit 4dfd2abebfe9d127d6361b195a9d06020c0318c9
Function Enter-PR-Parameters {
    Clear-Variable PrBodyContentReply
    $PrBodyContent = Get-Content -Path "$env:USERPROFILE\Github\winget-pkgs\.github\PULL_REQUEST_TEMPLATE.md"

    ForEach ($_ in ($PrBodyContent | Where-Object { $_ -like '-*[ ]*' })) {
        if ($_ -like "*winget validate*") {
            $PrBodyContentReply += $_.Replace("[ ]","[X]"), "`n"
        } else {
            Write-Host
            Write-Host
            Write-Host -ForegroundColor 'White' $_.TrimStart("- [ ]")
            Write-Host -ForegroundColor 'White' -NoNewline "[Y] Yes  "
            Write-Host -ForegroundColor 'Yellow' -NoNewline "[N] No "
            Write-Host -NoNewline "(default is 'N'): "
            do {
                $keyInfo = [Console]::ReadKey($false)
            } until ($keyInfo.Key)
            if ($keyInfo.Key -eq 'Y') {
                $PrBodyContentReply += $_.Replace("[ ]","[X]"), "`n"
            } else {
                $PrBodyContentReply += $_, "`n"
            }
        }
    }

    Write-Host
    Write-Host
    Write-Host -ForegroundColor 'White' "Does this pull request resolve any issues?"
    Write-Host -ForegroundColor 'White' -NoNewline "[Y] Yes  "
    Write-Host -ForegroundColor 'Yellow' -NoNewline "[N] No "
    Write-Host -NoNewline "(default is 'N'): "
    do {
        $keyInfo = [Console]::ReadKey($false)
    } until ($keyInfo.Key)
    if ($keyInfo.Key -eq 'Y') {
        Write-Host
        Write-Host "Enter issue number. For example`: 21983, 43509"
        $ResolvedIssues = Read-Host -Prompt 'Resolved Issues' | TrimString
        Foreach($i in ($ResolvedIssues.Split(","))) {
            $PrBodyContentReply += "Resolves #$i`n"
        }
    } else {Write-Host}

    Write-Host
    $PrBodyContentReply = ($PrBodyContentReply.Trim() -ne '')
    $PrBodyContentReply

    #Set-Content -Path PrBodyFile -Value $PrBodyContentReply | Out-Null
    #gh pr create --body-file PrBodyFile -f
    #Remove-Item PrBodyFile
}

I am completely in favour of this. It will simplify the code much but for any first-time contributor who will be using this script, won't he will be going through a kind of weird prompts?

Fixed in commit 4f24c84... Now i think everything is perfect...

What, I swear I tested that and it had no effect... Probably broke something on my end

won't he will be going through a kind of weird prompts?

Yeeah it looks ugly, you can make a elseif for a custom prompt like the winget validate part, also if user selected yes to test in Sandbox the winget install prompt should be checked automatically with a elseif

Fixed in commit 4f24c84... Now i think everything is perfect...

What, I swear I tested that and it had no effect... Probably broke something on my end

I had tested PR Parameters feature on PS 5.1. What version are you currently using? I don't think it should break. Here is what I have tested.

Yeeah it looks ugly, you can make a elseif for a custom prompt like the winget validate part, also if user selected yes to test in Sandbox the winget install prompt should be checked automatically with a elseif

Will be making the changes and committing them soon. Thanks for the suggestion.

I use the Default Powershell (5.1.19041.1023) which comes with Windows 10 for compatibility, but our code is a tiny bit different since you haven't implemented the simplified prompts yet 😁 so we'll see where i messed up once you implement it since i believe you need to do the variable part differently👍

Function Enter-PR-Parameters {
    Clear-Variable PrBodyContentReply
    $PrBodyContent = Get-Content -Path "$env:USERPROFILE\Github\winget-pkgs\.github\PULL_REQUEST_TEMPLATE.md"

    ForEach ($_ in ($PrBodyContent | Where-Object { $_ -like '-*[ ]*' })) {
        if ($_ -like "*winget validate*") {
            $PrBodyContentReply += $_.Replace("[ ]","[X]"), "`n"
        } else {
            Write-Host
            Write-Host
            Write-Host -ForegroundColor 'White' $_.TrimStart("- [ ]")
            Write-Host -ForegroundColor 'White' -NoNewline "[Y] Yes  "
            Write-Host -ForegroundColor 'Yellow' -NoNewline "[N] No "
            Write-Host -NoNewline "(default is 'N'): "
            do {
                $keyInfo = [Console]::ReadKey($false)
            } until ($keyInfo.Key)
            if ($keyInfo.Key -eq 'Y') {
                $PrBodyContentReply += $_.Replace("[ ]","[X]"), "`n"
            } else {
                $PrBodyContentReply += $_, "`n"
            }
        }
    }

    Write-Host
    Write-Host
    Write-Host -ForegroundColor 'White' "Does this pull request resolve any issues?"
    Write-Host -ForegroundColor 'White' -NoNewline "[Y] Yes  "
    Write-Host -ForegroundColor 'Yellow' -NoNewline "[N] No "
    Write-Host -NoNewline "(default is 'N'): "
    do {
        $keyInfo = [Console]::ReadKey($false)
    } until ($keyInfo.Key)
    if ($keyInfo.Key -eq 'Y') {
        Write-Host
        Write-Host "Enter issue number. For example`: 21983, 43509"
        $ResolvedIssues = Read-Host -Prompt 'Resolved Issues' | TrimString
        Foreach($i in ($ResolvedIssues.Split(","))) {
            $PrBodyContentReply += "Resolves #$i`n"
        }
    } else {Write-Host}

    Write-Host
    $PrBodyContentReply = ($PrBodyContentReply.Trim() -ne '')
    $PrBodyContentReply

    #Set-Content -Path PrBodyFile -Value $PrBodyContentReply | Out-Null
    #gh pr create --body-file PrBodyFile -f
    #Remove-Item PrBodyFile
}

I recently discovered that it will omit the last two lines of the template, aren't they important. If not, I will be cleaning up and testing the Function and commit it.
Note: `<path>` is the name of the directory containing the manifest you're submitting.

-----

Naaaaaah tbh it looks cleaner without the 2 last lines, the note is just to inform how to do winget install and winget validate
And the ----- like have no function, and if we type something above it the text will be huge 🤦♂
BIG TEXT
-----

Normal text

-----

Let me know when its implemented :)

Done in commit 9b5cdbded23493fe18b3f513d76371e0ad5db077

Uhoh something gone wrong
https://github.com/microsoft/winget-pkgs/pull/23909
You need a function like this somewhere i believe
$PrBodyContentReply = ($PrBodyContentReply.Trim() -ne '')

Oh! actually, I wrote the code in Untitled.ps1 and copied it here. error in copying the code 🤦♂

https://desktop.githubusercontent.com/releases/2.4.3-539849ed/GitHubDesktopSetup.msi I think it should be removed.

https://desktop.githubusercontent.com/releases/2.4.3-539849ed/GitHubDesktopSetup.msi I think it should be removed.

This XML file does not appear to have any style information associated with it. The document tree is shown below.
<Error>
ResourceNotFound
<Message>The specified resource does not exist. RequestId:cb182efa-d01e-004f-0a1a-8a5a9d000000 Time:2021-08-05T16:51:13.2113922Z</Message>
</Error>

But if you are talking about the GitHub Desktop Machine Wide installer we're waiting for Schema v1.1.0

No.. Download URL is invalid in manifests. So I am removing the manifests.

Uhhh i just checked a few URLs and they all fail up to the latest 2.9.0
https://desktop.githubusercontent.com/releases/2.9.0-4806a6dc/GitHubDesktopSetup-x64.msi and https://desktop.githubusercontent.com/releases/2.9.0-4806a6dc/GitHubDesktopSetup-x64.exe

Uhhh i just checked a few URLs and they all fail up to the latest 2.9.0
https://desktop.githubusercontent.com/releases/2.9.0-4806a6dc/GitHubDesktopSetup-x64.msi and https://desktop.githubusercontent.com/releases/2.9.0-4806a6dc/GitHubDesktopSetup-x64.exe

What should we do?
Remove all manifests...
version 2.9.0 is accessible through the browser...

Yup seems like they changed the URL's and the older versions are still not accessable
https://desktop.githubusercontent.com/github-desktop/releases/2.9.0-4806a6dc/GitHubDesktopSetup-x64.exe
https://desktop.githubusercontent.com/github-desktop/releases/2.9.0-4806a6dc/GitHubDesktopSetup-x64.msi
Guess it dosn't hurt to purge the package

Yup seems like they changed the URL's and the older versions are still not accessable
https://desktop.githubusercontent.com/github-desktop/releases/2.9.0-4806a6dc/GitHubDesktopSetup-x64.exe
https://desktop.githubusercontent.com/github-desktop/releases/2.9.0-4806a6dc/GitHubDesktopSetup-x64.msi
Guess it dosn't hurt to purge the package

What should I do finally, remove manifests or change URLs? Levvie approved them so fast!

Check if inserting /github-desktop/ in the URL works, if not delete them

@Trenly you didn't approve this one?

Any method to get PFN of MSIX/APPX file using a command?

PFN?
Also about FamilyName and SignatureSha, i've searched around for this before, and from what i found is that it has very little information and support, FamilyName can be found by installing the software and doing get-appxpackage, SignatureSha i have no idea how to read, but for some reason they built it into winget hash

Yes, I want to get the package family name without installing, any way to do that?

@vedantmgoyal2009 as you make improvements/suggestions to YamlCreate, would you mind considering adding new features to the Windows Package Manager Manifest Creator? We will be making this an optional package for the Windows Package Manager so one can winget create. We do want to have full fidelity. There may already be sample code for doing some of the things you're asking about.

The wingetcreate.exe detects many of these properties.

PFN?

Abbreviation for "Package Family Name"

Also about FamilyName and SignatureSha, i've searched around for this before, and from what i found is that it has very little information and support, FamilyName can be found by installing the software and doing get-appxpackage, SignatureSha i have no idea how to read, but for some reason they built it into winget hash

We can at least give a prompt if a user knows how to find it/already have it, then he can fill it while creating the manifests. A while ago, I had to create a manifest in which SignatureSha265 and PackageFamilyName have to be filled but since YamlCreate does not prompt for it, I had to edit and add them manually. If in future, somebody finds a method to automatically get it, we can show it like in ProductCode we do.
Product Code found from the installer: <automatically detected product code>

@vedantmgoyal2009 as you make improvements/suggestions to YamlCreate, would you mind considering adding new features to the Windows Package Manager Manifest Creator? We will be making this an optional package for the Windows Package Manager so one can winget create. We do want to have full fidelity. There may already be sample code for doing some of the things you're asking about.

I would definitely like to contribute to wingetcreate but it will take some time as I will have to understand C#. I currently do not know even 'a' of CSharp.

Do this https://github.com/Trenly/winget-pkgs/pull/4#pullrequestreview-724810079, i can't suggest changes since its outside of the changed scope


Webpack - Unable to make custom Addons

Uncaught TypeError: Cannot read property 'src/js/templates/core-empty-line.hbs' of undefined
The error occurs in Core.prototype.clean function, specifically this line:

this.$el.html(this.templates'src/js/templates/core-empty-line.hbs'.trim());

Here's a snapshot of the "this" value:

Background:

Webpack setup follows this guide: https://github.com/orthes/medium-editor-insert-plugin/wiki/v2.x-Using-with-webpack
Only modification I made was to bind window.MediumInsert to the require('medium-editor-insert-plugin')($); statement
This was an attempt to get access to window.MediumInsert.Templates, as shown in the CustomAddon  function https://github.com/orthes/medium-editor-insert-plugin/wiki/v2.x-Custom-addons

Any help would be very appreciated! This is an awesome plugin, just having some difficulty creating a custom addon to add an iframe to a page.

I had the same problem with webpack. Try this:
     import MediumInsert from 'medium-editor-insert-plugin'
     window.MediumInsert = MediumInsert.MediumInsert

This one worked for me ...
import *  as MediumInsert from 'medium-editor-insert-plugin/dist/js/medium-editor-insert-plugin';
window["MediumInsert"] = MediumInsert.MediumInsert;

Although, using this plugin in webpack is nightmare. Please include some documentation on how to add this in webpack.


BUG: Mention Cloudflare Pitfalls and Discrepancies in SST docs.

PITFALL 1:
Please mention somewhere in SST docs that initializing outside of a function in Cloudflare workers is going to cause issues.
/*
This is merely an EXAMPLE to demo a problem - do NOT publish your secrets on a public API
*/

// CORRECT WAY

function get_some_secret() {
    return Resource.SomeSecret.value;
}

export default {
    async fetch(request: Request, env: any, ctx: any) {
        return new Response(JSON.stringify({"config_val": get_some_secret()})), {
            headers: { 'Content-Type': 'application/json'  },
        });
    },
};

// WRONG WAY

const x = Resource.SomeSecret.value;

export default {
    async fetch(request: Request, env: any, ctx: any) {
        return new Response(JSON.stringify({"config_val": x})), {
            headers: { 'Content-Type': 'application/json'  },
        });
    },
};

BUG 1:
Linking with sst.cloudflare.Queue simply doesn't work and returns the following error(s).
export const queue = new sst.cloudflare.Queue("MyQueue");

export const worker = new sst.cloudflare.Worker("MyWorker", {
  handler: "packages/functions/src/some_worker_handler.ts",
  url: true,
  link: [queue],
});

~  Deploy

|  Error       MyWorkerScript cloudflare:index:WorkerScript
|  cloudflare:index/workerScript:WorkerScript resource 'MyWorker
Script' has a problem: Missing required argument. The argument "
queue_binding.0.binding" is required, but no definition was foun
d.. Examine values at 'MyWorkerScript.queueBindings'.
|  Error       MyWorkerScript cloudflare:index:WorkerScript
|  cloudflare:index/workerScript:WorkerScript resource 'MyWorker
Script' has a problem: Invalid or unknown key. Examine values at
 'MyWorkerScript.queueBindings'.

✕  Failed
   MyWorkerScript cloudflare:index:WorkerScript
   cloudflare:index/workerScript:WorkerScript resource 'MyWorker

did you figure this out? I suspect it's because the Cloudflare Queue construct has not been released officially
alternatively, do you know if it's possible / how to create other Cloudflare constructs (e.g. Workflows or Vectorize or Queue) and link to sst.config.ts? e.g.

@zxt-tzx Nope, can't instantiate other Cloudflare constructs like Workflows/Vectorize/Queue etc. because you can't use your own wrangler.toml file (use to define infra. components you need).
TBH SST is more trouble than worth for Cloudflare. Best for AWS it seems...

We are going to take a pass at some of the Cloudflare stuff soon. We need a few things from their side for it.

We are going to take a pass at some of the Cloudflare stuff soon. We need a few things from their side for it.

awesome, can't wait. do you have a rough timeline?

The queue issue is with the bindings. They have inconsistent naming
See https://github.com/sst/sst/blob/3bd0e950f56ca7553ed699c23e4434b7c7400729/platform/src/components/cloudflare/worker.ts#L335C11-L338C16
   if (!result[b.binding]) result[b.binding] = [];
            result[b.binding].push({
              name,
              ...b.properties,
            });

The code assumes that each binding type has a name property (bindings). However, if you see the Cloudflare pulumi docs (https://www.pulumi.com/registry/packages/cloudflare/api-docs/workersscript/#constructor-example) , there is inconsistency in parameters
d1DatabaseBindings: [{
        databaseId: "string",
        name: "string",
    }],
    hyperdriveConfigBindings: [{
        binding: "string",
        id: "string",
    }],
    kvNamespaceBindings: [{
        name: "string",
        namespaceId: "string",
    }],
    analyticsEngineBindings: [{
        dataset: "string",
        name: "string",
    }],

    plainTextBindings: [{
        name: "string",
        text: "string",
    }],
    queueBindings: [{
        binding: "string",
        queue: "string",
    }],
    r2BucketBindings: [{
        bucketName: "string",
        name: "string",
    }],
    secretTextBindings: [{
        name: "string",
        text: "string",
    }],
    serviceBindings: [{
        name: "string",
        service: "string",
        environment: "string",
    }],

Sometimes it's name, sometimes binding

Next thing regarding the queue, is that it expects the name instead of ID
Current properties - https://github.com/sst/sst/blob/6f626051d584609217e5d52e398ec6a6080b32ce/platform/src/components/cloudflare/queue.ts#L59
The queue binding
export interface WorkerScriptQueueBinding {
    /**
     * The name of the global variable for the binding in your Worker code.
     */
    binding: string;
    /**
     * Name of the queue you want to use.
     */
    queue: string;
}

So, if you do it like this:
  getSSTLink() {
    return {
      properties: {},
      include: [
        binding({
          type: "queueBindings",
          properties: {
            queue: this.queue.name,
          },
        }),
      ],
    };
  }

It works

I opened a pr https://github.com/sst/sst/pull/5045 with fixes. Now you can enqueue entries from the worker. However, as far as I know, there is no way to create a queue consumer using SST
So in example like this
import { Resource } from "sst";
import { ExportedHandler } from "@cloudflare/workers-types"

export default {
  /**
   * {@link https://developers.cloudflare.com/queues/get-started/ | CF queue guide }
   */
  async fetch(request, env, ctx): Promise<Response> {
    let log = {
      url: request.url,
      method: request.method,
      headers: Object.fromEntries(request.headers),
    };
    await Resource.LogQueue.send(log);
    return new Response('Success!');
  },
  async queue(batch, env): Promise<void> {
    let messages = JSON.stringify(batch.messages);
    console.log(`consumed from our queue: ${messages}`);
  },
} satisfies ExportedHandler;

You can add items in the fetch function, but you can't consume them in the queue function


cached modules cause problems due to changed filepath

k's java-backend caches compiled modules including the filepath to that module across executions. If now the module is compiled again from a different location without being changed the old filepath is used through the execution. Which causes problems if the old file is deleted.
A solution could be an additional flag which prevents k from caching compiled modules across executions or k rebuilding modules if its path changed.

This is more generally because k's caching relies on Object.equals which was not implemented to compare attributes. We have similar problems with the integrity of the cache at compile time. The solution is to compare attributes when caching, but it might take some effort to get everything to work properly when we make the change.
The workaround for now is to delete the kompiled directory and recompile the definition. I will look into fixing this when I get back to the office next week.

I was not able to reproduce this behavior; can you provide me with a sequence of steps that you can reproduce the issue with if you want this resolved?

KLab's cacheing (and K's) has changed significantly since this issue was opened, so it's not likely we can reproduce it now. If it's still an issue @mhhf feel free to re-open with an example of the problem on latest K/KEVM/KLab.


Source map has broken in 3.x?

I've just updated gulp-sass to 2.x (which updates node-sass to 3.x), and my source map has completely changed:
-{"version":3,"sources":["test.scss"],"names":[],"mappings":"AAGK,UAHG,MAAA","file":"test.css","sourcesContent":["$color: blue;\n\nbody {\n  .div {\n    color: $color;\n  }\n}\n"],"sourceRoot":"/source/"}
+{"version":3,"sources":["test.css"],"names":[],"mappings":"AAAA,UACE,MAAA","file":"test.css","sourcesContent":["body .div {\\n  color: blue; }\\n"],"sourceRoot":"/source/"}

I don't think this is an issue with gulp-sass, but I'll cc @Snugug who updated gulp-sass anyway.
Any ideas?

You're content has changed too, as we can see by looking at sourcesContent!?!?

Yep. That's the output!
Literally the only change I made was updating gulp-sass, input and environment are the same.

Yep, and of course the output position is part of the source-map, therefore the change!
You may want to inspect the result further here: http://murzwin.com/base64vlq.html

@mgreter I think what @callumacrae is getting at is that he has not changed the source content. Instead now the source content is showing the compiled css not the source Sass. This sounds like a bug.

@callumacrae can you test whether this bug happens when using node-sass directly so we can determine if it's a Libsass, node-sass or gulp-sass issue.

var sass = require('gulp-sass/node_modules/node-sass');

sass.render({
  file: 'test/fixtures/sass/test.scss',
  outFile: 'test.css',
  sourceMap: true
}, function (err, res) {
  if (err) {
    throw err;
  }

  console.log(res.css.toString());
  console.log(res.map.toString());
});

output:
body .div {
  color: blue; }

/*# sourceMappingURL=test.css.map */
{
	"version": 3,
	"file": "test.css",
	"sources": [
		"test/fixtures/sass/test.scss"
	],
	"sourcesContent": [],
	"mappings": "AAGE,IAAI,CAAC,IAAI,CAAJ;EACH,KAAK,EAJD,IAAI,GAGJ",
	"names": []
}

Seems to be a bug in gulp-sass, then. I'll open an issue over there.

@callumacrae I'm confused. The mappings appear to be different again. I don't expect gulp-sass is altering the mappings. It would appear that you are proving output from multiple versions of node-sass.
Also I don't believe sourceContent should be empty.

When it was ran through gulp, it was ran through other stuff like autoprefixer and a minifier. gulp-sass is the only thing that changed between the two maps in the first post, so it couldn't have been anything else that broke it.

As in the issue on gulp-sass, it's an issue over there; I'll close this one.
https://github.com/dlmanning/gulp-sass/issues/269


Issues with compose preview

Really great library just what I was looking for and the demo code was also really helpful but issues with compose preview, when we click on button to open the bottom sheet, it doesnt opens rather the button keeps getting darker where we touch it everytime

Hi, thx 🤗
ModalSheet is built on FullscreenPopup, which is a lightweight version of Compose Popup and as for now Compose does not support multi-window previews. For example this is a reason why Popup previews also do not work.
I would recommend similar approach as in this SO answer, just extract ModalSheet content to the separate Composable and create a preview for it.
https://stackoverflow.com/a/69681577/14743614


Kubernetes: Add support for rootless images

fix #1077
fix #3552
fix #3164

[ ] Docs

Background
The blocking issue for non-root images in k8s are the volume mount permissions as noted by @6543 in https://github.com/woodpecker-ci/woodpecker/issues/1077#issuecomment-1901163825.
Volumes are by default owned by root:root. This can be changed for the group using securityContext.fsGroup. However, only the mount point of the volume has 775 permissions, i.e. the option for the group to write.
WP creates <mount point>/$CI_WORKSPACE internally by setting workingDir in https://github.com/woodpecker-ci/woodpecker/blob/a5448360b6c002463a6e521cbb19eac4a45f9806/pipeline/backend/kubernetes/pod.go#L185.
This directory gets created with root:<fsgroup> but with 755, i.e. the group can't write there, even with  being set correctly.
Solution
Setting workingDir to workspaceBase, i.e. /woodpecker, and creating <workspaceBase>/src/<repo slug> via the container user solves this. The subsequently created dir is then owned by the container user and all operations work.
To achieve this, workspaceBase has been added to the Step struct and set as workingDir during pod creation.
Sidework
The hard-coded $HOME is an additional issue here. Removing it and letting the container define $HOME is the most straightforward solution. 99% of all containers have $HOME defined and I don't see any downside from removing the hard-coded setting, especially as it can't be overwritten within environment: even.
SecurityContext
Most non-root images run with UID=1000 and GID=1000. Hence, I argue that we should set fsGroup: 1000 as the default (coupled with fsGroupChangePolicy: "OnRootMismatch") and document that users need to change that backend option in case their image user has a different value. This is coupled with fsGroupChangePolicy, which I added as a new SecurityContext param, but only if fsGroup is also set.
Setting runAsUser is not helpful as it would conflict with rootful images. Though, because non-root images are making use of the group write permissions, setting fsGroup should be enough.
Acknowledgements
I learned a lot about workspaceBase and workspacePath in https://github.com/woodpecker-ci/woodpecker/issues/2510, thanks @zc-devs and @dominic-p for the discussion!
Reprex
when:
  event:
    - push

skip_clone: true

steps:
  test:
    image: renovate/renovate
    commands:
      - whoami
      - pwd
      - echo $HOME
      - ls -la /woodpecker
      - echo $CI_WORKSPACE
      - ls -la $CI_WORKSPACE
      - mkdir /woodpecker/test
      - ls -la /woodpecker/test

yields
+ whoami
ubuntu
+ pwd
/woodpecker/src/<redacted>/$CI_REPO
+ echo $HOME
/home/ubuntu
+ ls -la /woodpecker
total 24
drwxrwsr-x. 4 root   ubuntu  4096 Oct  2 08:31 .
drwxr-xr-x. 1 root   root      26 Oct  2 08:31 ..
drwxrws---. 2 root   ubuntu 16384 Oct  2 08:31 lost+found
drwxr-sr-x. 3 ubuntu ubuntu  4096 Oct  2 08:31 src
+ echo $CI_WORKSPACE
/woodpecker/src/<redacted>/$CI_REPO
+ ls -la $CI_WORKSPACE
total 8
drwxr-sr-x. 2 ubuntu ubuntu 4096 Oct  2 08:31 .
drwxr-sr-x. 3 ubuntu ubuntu 4096 Oct  2 08:31 ..
+ mkdir /woodpecker/test
+ ls -la /woodpecker/test
total 8
drwxr-sr-x. 2 ubuntu ubuntu 4096 Oct  2 08:31 .
drwxrwsr-x. 5 root   ubuntu 4096 Oct  2 08:31 ..

Using "clone" plugin
+ whoami
ubuntu
+ pwd
/woodpecker/src/<redacted>/$CI_REPO
+ echo $HOME
/home/ubuntu
+ ls -la /woodpecker
total 24
drwxrwsr-x. 4 root ubuntu  4096 Oct  2 13:42 .
drwxr-xr-x. 1 root root      26 Oct  2 13:42 ..
drwxrws---. 2 root ubuntu 16384 Oct  2 13:42 lost+found
drwxrwsr-x. 3 root ubuntu  4096 Oct  2 13:42 src
+ echo $CI_WORKSPACE
/woodpecker/src/<redacted>/$CI_REPO
+ ls -la $CI_WORKSPACE
total 24
drwxrwsr-x. 4 root ubuntu 4096 Oct  2 13:42 .
drwxrwsr-x. 3 root ubuntu 4096 Oct  2 13:42 ..
drwxrwsr-x. 9 root ubuntu 4096 Oct  2 13:42 .git
drwxrwsr-x. 2 root ubuntu 4096 Oct  2 13:42 .woodpecker
-rw-rw-r--. 1 root ubuntu   60 Oct  2 13:42 Dockerfile
-rw-rw-r--. 1 root ubuntu  854 Oct  2 13:42 build.yaml
+ mkdir /woodpecker/test
+ ls -la /woodpecker/test
total 8
drwxr-sr-x. 2 ubuntu ubuntu 4096 Oct  2 13:42 .
drwxrwsr-x. 5 root   ubuntu 4096 Oct  2 13:42 ..

well instead of removing it, we should add a check if it was not set and then set it accordingly
and HOME should not be WORKDIR in normal cases ... (as tools often create/alter (config-)files in HOME)

well instead of removing it, we should add a check if it was not set and then set it accordingly

What is "accordingly"? I don't see why we would need to do that - why not let the image decide it?

and HOME should not be WORKDIR in normal cases

It isn't anyways? WORKDIR is /woodpecker/src/<repo slug>? (= $CI_WORKSPACE)

Most non-root images run with UID=1000 and GID=1000

Considering the Bitnami's images alone, the statement is too strong (already).

We won't be able to cover all GIDS, yeah, but I think 1000 is the best possible default? And better than having none.

We won't be able to cover all GIDs, but I think 1000 is the best possible default? And better than having none.

⬇

We won't be able to cover all UIDs, but I think 1000 is the best possible default? And better than having none.

??? :)
I gave you an example of widely used images (IMO, the second ones after official), which will likely be broken by the default settings from this PR. There is a lot other images, that use group other than 1000, I believe. In case of Bitnami, curious person can start from their 9485 issue.

Let me share my thoughts.
We have the workspace. So, the base is the volume mount point, some sub-directories can exist there.
One of them is sources. I think, clone plugin is responsible for creating this sub-directory and set appropriate permissions.

The workspace defines the ... working directory shared by all workflow steps.

So, prerequisites are:

Source directory needs to be shared by all steps
We do not know what user/group step would run under

The third set of permissions is generally referred to as "others". So, clone plugin should set permissions like
chmod -R 777 /woodpecker/src/
# or
chmod -R 666 /woodpecker/src/

In regards volume mount point...

The workspace defines the volume shared by all workflow steps

The logic and prerequisites are the same: something should grant w (probably x) permissions to others on volume root. Obviously, fsGroup doesn't work here. This "some thing" should be the first step in a workflow. So, it could be some "volume initialization" step. Or this responsibility could be delegated to clone plugin (temporarily?).

I'm sorry for giving non-asked feedback, BTW.

??? :)

UIDs are not of interest. The mount point needs to have group w to initialize CI_WORKSPACE with user w.

I gave you an example of widely used images (IMO, the second ones after official), which will likely be broken by the default settings from this PR.

Why would they? AFAIU you can't run non-root bitnami images right now in k8s and execute arbitrary steps due to the apparent limitations. Even when adapting securityContext etc. With this PR, you can (for the first time)? Stating that they're broken with this PR infers that they would be working right now?

There is a lot other images, that use group other than 1000, I believe.

🤷 We can only set one default value and we won't be able to cover all non-root variants. A dynamic setting is also not possible AFAICS. Setting a default here is better than none? Do you have a better suggestion?

So, it could be some "volume initialization" step. Or this responsibility could be delegated to clone plugin (temporarily?).

Why? I don't see that we would need that. And I also would prefer to avoid adding an init container or similar.
Is the current logic fully clear?

We ensure group w on the volume mount point (=workingDir)
Create the actual workingDir (src/) within the container.
Have a functional workingDir with user w and group w

In my tests so far, I couldn't find a use case yet for which it doesn't work or for which it would break something else. Still needs more testing of course, but I am optimistic so far, that this is a good solution.

Stating that they're broken with this PR infers that they would be working right now?

I do not know and lazy to test :) But point was ⬇

Setting a default here is better than none? Do you have a better suggestion?

So, you've introduced some options that allow to run any non-root image, right? OK, that is good 👍
But why do you want to set default? Why 1000, why not 1001, 100, 101, 65532?
I think, there should not be the default value, the same as for runAsUser and runAsGroup. Or it should be optional (agent?) setting.

I couldn't find a use case yet for which it doesn't work

Plugins, which rely on current behavior, will probably be broken.
		WorkingDir:      step.WorkSpaceBase,

workspace:
  base: /go
  path: cache # who creates this directory? it was container runtime previously.

steps:
  build:
    image: go-build-plugin
    settings:
      some: value

https://github.com/woodpecker-ci/woodpecker/issues/1077#issuecomment-1901163825

steps:
  build:
    image: alpine-build-plugin
    settings:
      apkbuild-dir: /woodpecker/src/repo/app/ # what owner/permissions was set there by clone plugin?
    backend_options:
      kubernetes:
        securityContext:
          runAsUser: buildozer
          runAsGroup: abuild

Would plugin has access to desired directory?

With the "clone" plugin, the perms are 775 on $CI_WORKSPACE

Seems, yes. But would abuild like the owner of "workdir"? ¯_(ツ)_/¯
There was some reason to fix the owner, but I do not remember now. Maybe it is not related here.

But why do you want to set default? Why 1000, why not 1001, 100, 101, 65532?

In some scenarios implicit defaults exists that are honored by a majority. IMO this applies here.
Using 1000 would mean we cover a large fraction of non-root images by default, without having to set a custom securityContext for every pipeline. This is better than having to do it for all images. Yes, nobody can give a exact number WRT to the share but again, why go with 0 if you can have more without issues?

https://github.com/woodpecker-ci/woodpecker/pull/4151#discussion_r1784099900, which rely on current behavior, will probably be broken.

Yes, the PR is breaking. Even though I'd say not many users overwrite workspace.

However, there is no write permissions in workspace for users other, than clone step runs.

Do you mean by non-root plugins that use a different non-root user?
This would apply yes, however right now all plugins are rootful AFAIK (something to address).
We could set 777 to workaround that. Given that the pod PVs are only used once during the steps, I don't see why this would be an issue.

Do you mean by non-root plugins that use a different non-root user?

Yes, but apparently, it applies to general steps also:
steps:
  test:
    image: alpine
    commands:
    - echo "hello" > touch /woodpecker/src/repo/temp.txt
    backend_options:
      kubernetes:
        securityContext:
          runAsUser: 101 # other than `clone` user
          runAsGroup: 101 # other than `clone` group

We could set 777 to workaround that

So, fsGroup stuff isn't needed then? :)

something should grant w (probably x) permissions to others on volume root. This "some thing" should be the first step in a workflow. This responsibility could be delegated to clone plugin (temporarily?)

"on volume root" && "workspace" (aka source directory).

So, fsGroup stuff isn't needed then? :)

It is (for skip_clone: true), because the first mkdir is needed with working group w on /woodpecker. We could chain a chmod 775 after this, so that $CI_WORKSPACE has 775 instead of 755.
Fun fact: the value of fsGroup is not important it seems, as the container user will automatically be member of the group referenced by fsGroup (I justed tested this with a nonsense value). The only important part is though: mkdir needs to work to be able to create $CI_WORKSPACE.
Scenario: skip_clone: true:

Use non-root image with GID 1000 -> $CI_WORKSPACE will be 755 owned by UID:GID of the first image.
For a second step, the existing volume would be mounted. Assuming a different non-root UID/GID now, no permissions on $CI_WORKSPACE would exist. So I guess forcing a chmod 775 right from the first creation of $CI_WORKSPACE makes sense.

-> Just tested this and it seems that using any value for fsGroup is sufficient. In step2, the group owner is adjusted and the image user is automatically part of that group, ensuring group w on $CI_WORKSPACE, recursively.

So based on all findings so far, defining fsGroup with an arbitrary value and ensuring workingDir = mount point seems to solve all non-root execution issues?

I now verified that NOT setting fsGroup won't work when using multiple non-root steps. An arbitrary value for fsGroup (I went with 1000 for the default now) works, even if using different images with different UID/GIDs of non-root users.

We could set 777 = something should grant w permissions to others

then you are talking about user/group permissions

775, 755

There are rx permissions for others. I'm lost...

An arbitrary value for fsGroup works, even if using different images with different UID/GIDs of non-root users.

I'm convinced, but have other questions:

Does Docker support fsGroup or something similar?
Performance. Each time (step), when fsGroup differ from previous, Kubernetes will change group of each file/dir. I doubt it would big hit to performance, but is it necessary at all, if we can just chmod -R o+w /woodpecker in clone step (first aka initialization) once?

There are rx permissions for others. I'm lost...

Not sure what you mean, but we need w - the diff between 5 and 7 is missing w.

but still need to test Docker and Local.

WRT to the workspaceBase changes, yes. Everything else is k8s-specific.

if we can just chmod -R o+w /woodpecker in clone step (first aka initialization) once?

The solution aims to be generic and cover skip_clone: true explicitly. Ideally, plugin-git would also run rootless and then fsGroup would be a hard requirement.
The PR does a few things which are all needed together to make non-root images work:

Remove the hard-coded $HOME (most important for the "docker" backend)
Ensure correct permissions on $CI_WORKSPACE for skip_clone: true and skip_clone: false
Ensure that all subsequent steps have write access to $CI_WORKSPACE, thanks to fsGroup

Does it work on Windows?

Most changes don't apply to it. The only ones that do, are related to $HOME again. It should I guess, but I can't and won't test it. Not even sure if anyone is using WP on Win? But this is a different discussion.

Not sure what you mean, but we need w - the diff between 5 and 7 is missing w

Yes

We could set 777 = something should grant w permissions to others

So, others is the last digit => last digit should be 6 or 7. chmod -R o+w /woodpecker in the first step.
I've explained already why others.

Most changes don't apply to it. The only ones that do, are related to $HOME again.

I was not about HOME in the question 3. Let me cite the docs:

The .spec.os.name field should be set to windows to indicate that the current Pod uses Windows containers.
If you set the .spec.os.name field to windows, you must not set the following fields in the .spec of that Pod:
spec.securityContext.fsGroup
spec.securityContext.fsGroupChangePolicy

and your code
// if unset, set fsGroup to 1000 by default to support non-root images

Each time changing group ownership of all files/dirs by using Kubernetes specific fsGroup, which doesn't work on Windows (therefore should not be default value)

Why is Windows in the discussion here? The fsGroup part does not touch Windows-related code, does it?

Or granting necessary permissions (rwx, all?) to others once in the first step (roughly speaking), which is universal solution

I explained this before: this won't work for skip_clone: true, at least the first step needs fsGroup to create $CI_WORKSPACE (or must be a rootful image (e.g. the clone plugin)). The latter shouldn't be needed necessarily and hence, yes, fsGroup is required at least for the first step. And as only setting it there would be complex and codewise and also possibly confuse users, I think we should just set it for every pod.
Using rootless images should ideally just work and not require any additional flags to enable it.

(Does Docker support fsGroup or something similar?).

Docker does not have the same volume restrictions k8s has. Running rootless in docker is much easier and should already work after the removal of hard-coded $HOME. Hence, the backend/docker label.

What mechanism does set write access in Docker backend? Or in Docker non-root steps work without write access to $CI_WORKSPACE?

Yes, docker is different (and simpler but also less secure). But this is a different topic now, I am out of words 🙃

HOME

it can't be overwritten within environment
instead of removing it, we should add a check if it was not set

I would extract Home overriding into separate PR:
If it was set in environment, then override default. Overwise, leave /root default (for now).

@woodpecker-ci/maintainers any comments/review?

Have to test it yet but my mine concern is permissions also not only for workspace folder but files used in later steps also, how will it work for mixed root/non-root steps

Root can always write.
For rootless, fsGroup takes care setting (group) permissions for every step. See previous discussions in this PR.

@anbraten @6543 @qwerty287 Review appreciated.

@xoxys Do you maybe have time for a full review?

the windows scripts are not addressed ... I'll have a look at them

@pat-s you have my go but plz look at my last changes if they work for u too


Issue: How to pass in database name parameter into Milvus

Issue you'd like to raise.

In Milvus, we are able to create databases and create collections within the database itself. Hence i will be able to create a collection with the same name in different databases. How can we specify the specific database and collection to use, currently the parameters only accept collection_name and not db_name like in pymilvus.
Suggestion:
No response

Meet same issue

we have this issue as well

i have this issue too

I also encountered the same problem, langchain always links to the default database. Any solution?

+1

set "db_name" in "connection_args".
object = Milvus(
    connection_args = {
        "db_name": "your db name"
    }

Tested in v0.1.0, the solution of @wheatfox works.

@ohbeep  Should this be added to this section the documentation? I can create a pull requests.

@ohbeep Should this be added to this section the documentation? I can create a pull requests.

I don't think so. This db_name parameter should be a key of the connection_args field.

drop_old is very expensive operation that you are using while building connection, It delete existing data in collection before making new collection

connection_args需要添加参数 alias
langchain 0.1.15 milvus.py _create_connection_alias()方法
        # If a valid address was given, then check if a connection exists
        if given_address is not None:
            for con in connections.list_connections():
                addr = connections.get_connection_addr(con[0])
                if (
                    con[1]
                    and ("address" in addr)
                    and (addr["address"] == given_address)
                    and ("user" in addr)
                    and (addr["user"] == tmp_user)
                ):
                    logger.debug("Using previous connection: %s", con[0])
                    return con[0]

        # Generate a new connection if one doesn't exist
        alias = uuid4().hex

con[0]没做空校验的，如果传入了alias参数，就应该在连接时指定alias，这里完全没去做校验，完全没法指定数据库，我如果部署了一套环境 ，使用数据库来区分环境，这个没法做到，希望官方后面版本修复这个问题

This should definitely by added:
Using MilvusClient from pymilvus, you can connect like this:
class MilvusClient(     uri: str = "http://localhost:19530",     user: str = "",     password: str = "",     db_name: str = "",     token: str = "",     timeout: float | None = None,     **kwargs: Any )
Here obviously, db_name is a parameter which one can use (and we did that already). Furthermore confirmed by the Attu-GUI, where we can see e.g. several databases, not just the default one...

Issue you'd like to raise.

In Milvus, we are able to create databases and create collections within the database itself. Hence i will be able to create a collection with the same name in different databases. How can we specify the specific database and collection to use, currently the parameters only accept collection_name and not db_name like in pymilvus.
Suggestion:
No response

LangChain doesn't support this feature directly, but you can achieve it using the following approach:
Import
from pymilvus import connections, db, utility
from langchain.vectorstores import Milvus
Connect to Milvus
CONNECTION_URI = "http://localhost:19530"
connections.connect(uri=CONNECTION_URI)
Define database name
db_name = "my_db"
Check if the database exists, and create it if it does not
db_list = db.list_database()
if db_name not in db_list:
db.create_database(db_name)
Example usage with Milvus
vector_db = Milvus.from_documents(
documents=text_chunks,
embedding=embeddings,
connection_args={
"uri": CONNECTION_URI,
"db_name": db_name
}
)

I can't believe specifying a database is not an option. This should definitely be an open / planned issue. Unfortunately, Haystack suffers from the same omission.


Prune --full CLI arg, doesn't have precedence over toml file

Describe the feature
Help printed for --full flag doesn't match the program. The toml file overwrites the cli args.
https://github.com/paradigmxyz/reth/blob/93ebdf292bf895c3948a3b446a69878fe4f66892/crates/node/core/src/args/pruning.rs#L12-L15
https://github.com/paradigmxyz/reth/blob/93ebdf292bf895c3948a3b446a69878fe4f66892/crates/node/builder/src/launch/common.rs#L349-L352
See discussion https://github.com/paradigmxyz/reth/pull/9782
Additional context
Config file overwriting cli args, is an anti-pattern we want to avoid, since it's unhelpful to users that know their way around cli, and rely on these standard patterns instead of reading each tools' help individually.
See https://www.gnu.org/software/gnuastro/manual/html_node/Configuration-file-precedence.html .

what we should do here is a merge function on Pruneconfig that combines the two values

blocks op-reth sync with --full flag

this describes the feature we're looking for here

blocks op-reth sync with --full flag

why is that?

since bug for chains without deposit contract didn't work before f175f6, the reth.toml already exists, and therefore won't be saved to reth.toml

unfortunately, remembered is still a problem for anyone trying to run an op full node already with their bedrock datadir, since then prune part will exist in reth.toml, hence won't be overwritten

Hey @emhane @mattsse is this still desired behavior? Happy to take this on if so.
FWIW agree that generally CLI args take precedence over config files

FYI @emhane I think this can be closed now that we've merged in https://github.com/paradigmxyz/reth/pull/10774


Update tab-text.js

Unable to set custom classname on Tab.Text

Coverage remained the same at 83.412% when pulling 003effc689102157f43b9a1273b4d09812b19435 on yoohahn:patch-1 into 36c160344bf0ec105670b1a3058868117f39ae15 on ricsv:master.


⚠ Samsung has degraded performance

In eaf5cdb, Samsung (https://samsung.com) experienced degraded performance:

HTTP code: 200
Response time: 2619 ms

Resolved: Samsung performance has improved in 2e94621.


Slow IPFS Cluster uploads

Shop deployments are sometimes happening pretty slow when using the IPFS cluster.  The cause is unknown.  Will document here.
From the @origin/ipfs-proxy logs:
ESC[37m2020-07-10T18:57:14.956Z ESC[32;22m[INFO]  ESC[33;1morigin-ipfs-proxyESC[0m: /add?name=franck-test-shop-a&recursive=true&stream-channels=true

From the dshop backend:
2020-07-10T18:57:13.384347931Z [INFO]  dshop.utils.deployShop: Preparing data for deploy...
2020-07-10T18:57:14.652193004Z [INFO]  dshop.utils.deployShop: IPFS cluster configured at /dns4/fs.ogn.app/tcp/443/https
/. Adding to the list of pinners.
2020-07-10T18:57:14.652254372Z [INFO]  dshop.utils.deployShop: Pinata configured. Adding to the list of pinners.
2020-07-10T18:57:14.65676707Z - 📦  Calculating size of /data/dshop-cache/rinkeby/franck-test-shop-a/public…
2020-07-10T18:57:14.664029743Z ✔ 🚚  Directory /data/dshop-cache/rinkeby/franck-test-shop-a/public weighs 9.2 MiB.
2020-07-10T18:57:14.693813973Z - 📠  Uploading and pinning to IPFS Cluster…

2020-07-10T18:59:58.28434307Z ✔ 📌  Added and pinned to IPFS Cluster with hash:
2020-07-10T18:59:58.285699851Z ℹ 🔗  QmQqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj (https://ipfs.io/ipfs/QmQqAjk6sAVWPr
LaQkmCsNZHSGxR32zngBfbHfU6DGRHBj/)
2020-07-10T18:59:58.286654785Z - 📠  Uploading and pinning to Pinata…
2020-07-10T19:00:05.903950201Z ✔ 📌  Added and pinned to Pinata with hash:
2020-07-10T19:00:05.90407807Z ℹ 🔗  QmQqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj (https://gateway.pinata.cloud/ipfs/Qm
QqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj/)
2020-07-10T19:00:05.904550895Z [INFO]  dshop.utils.deployShop: Deployed shop to 12 pinners. Hash=QmQqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj
2020-07-10T19:00:05.904588508Z [INFO]  dshop.utils.deployShop: Priming gateways...
2020-07-10T19:00:05.920669258Z [INFO]  dshop.utils.primeIpfs: Priming https://gateway.ipfs.io/ipfs/QmQqAjk6sAVWPrLaQkmCs
NZHSGxR32zngBfbHfU6DGRHBj/app.1da0c384.css

IPFS Cluster node 0
2020-07-10T18:59:56.666Z        ESC[31mERRORESC[0m      p2p-gorpc       go-libp2p-gorpc@v0.1.0/call.go:64       msgpack decode error [pos 0]: stream reset
2020-07-10T18:59:56.667Z        ESC[31mERRORESC[0m      adder   adder/util.go:58        BlockPut on 12D3KooWM1JxwDj3g7dBTtmuKvpD4xgHsaJ4Ze97R4rtF5Wm38wL: msgpack decode error [pos 0]: stream reset
2020-07-10T18:59:58.115Z        ESC[34mINFOESC[0m       cluster ipfs-cluster/cluster.go:1416    pinning QmQqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj on [12D3KooWM1JxwDj3g7dBTtmuKvpD4xgHsaJ4Ze97R4rtF5Wm38wL QmVR5ucBjneGoCv73tNww4HBu2vqUNjDpLNne7yGRYbDfz]:
2020-07-10T18:59:58.255Z        ESC[34mINFOESC[0m       crdt    crdt/consensus.go:210   new pin added: QmQqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj
2020-07-10T18:59:58.257Z        ESC[34mINFOESC[0m       crdt    go-ds-crdt@v0.1.12/heads.go:82  replacing DAG head: bafybeihtwbcr62msw7qjllyipzq4gtj26sam4fuf46zn54mt2bop6b6ijm -> QmPuS4n45Fp1KvDsG2o1J3ds35bUmmiCRRXY2b3SPf6M4G (new height: 322628)
2020-07-10T18:59:58.265Z        ESC[34mINFOESC[0m       adder   adder/adder.go:182      QmQqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj successfully added to cluster
2020-07-10T18:59:58.265Z        ESC[34mINFOESC[0m       restapilog      rest/restapi.go:113     <IP_ADDRESS> - - [10/Jul/2020:18:57:14 +0000] "POST /add?name=franck-test-shop-a&recursive=true&stream-channels=true HTTP/1.1" 200 12672

2020-07-10T19:00:00.004Z        ESC[34mINFOESC[0m       ipfshttp        ipfshttp/ipfshttp.go:378        IPFS Pin request succeeded: QmQqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj

IPFS Cluster node 1
2020-07-10T19:00:02.577Z        ESC[34mINFOESC[0m       crdt    crdt/consensus.go:210   new pin added: QmQqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj
2020-07-10T19:00:02.580Z        ESC[34mINFOESC[0m       crdt    go-ds-crdt@v0.1.12/heads.go:82  replacing DAG head: bafybeihtwbcr62msw7qjllyipzq4gtj26sam4fuf46zn54mt2bop6b6ijm -> bafybeiaxighlk7vsxccekczilx4xwmdwr4oemmcjlhxl7amfhr53ebdpwu (new height: 322628)
2020-07-10T19:00:03.982Z        ESC[34mINFOESC[0m       ipfshttp        ipfshttp/ipfshttp.go:378        IPFS Pin request succeeded: QmQqAjk6sAVWPrLaQkmCsNZHSGxR32zngBfbHfU6DGRHBj

This is very intermittent.  Everything is superfast again.  Could be load issues?  Gateway usage?  p2p traffic(which is generally ridiculous)?
Here's some baseline stats while everything is smooth(docker may skew these, unsure):
# lsof | wc -l
7

System stats from top:
Mem: 27749740K used, 3130748K free, 3408K shrd, 263048K buff, 12433192K cached
CPU: 17.0% usr  1.8% sys  0.0% nic 80.4% idle  0.0% io  0.0% irq  0.4% sirq
Load average: 5.60 4.02 3.20 3/2233 49

iostat -k:
Linux 4.14.138+ (prod-ipfs-cluster-0)   07/13/20    _x86_64_    (8 CPU)

avg-cpu:  %user   %nice %system %iowait  %steal   %idle
           5.28    0.00    2.08    0.05    0.00   92.59

Device:            tps    kB_read/s    kB_wrtn/s    kB_read    kB_wrtn
sda               2.62         9.08        26.28   70055516  202750797
dm-0              0.34         3.42         0.00   26402256          0
sdb               0.02         0.06         0.12     449341     902764
sdc               0.00         0.01         0.01      76369      41932
sdd               0.00         0.00         0.00      28969       7996
sde               0.00         0.00         0.00       8714      38320
sdf               0.01         0.02         0.03     159382     198932
sdg               0.50         1.08         5.17    8371394   39906704
sdh               0.83        26.40        40.89  203686430  315513324
sdi               0.00         0.00         0.00       3189         12
sdk               0.01         0.11         0.02     866853     138492

When it's next reported, maybe there will be a difference in these.

Could be unrelated, but this suggests there may have been connectivity issues between the peers at some point(seems unlikely)?  This was seen a couple of times in the logs:
2020-07-11T10:58:17.719Z INFO cluster ipfs-cluster/cluster.go:487 reconnected to QmVR5ucBjneGoCv73tNww4HBu2vqUNjDpLNne7yGRYbDfz

There doesn't appear to be a message signalling the disconnect.  Also this, which doesn't make a lot of sense.  The ID is ipfs-cluster-0:
2020-07-11T02:51:33.249Z WARN cluster ipfs-cluster/cluster.go:400 metric alert for freespace: Peer: QmVR5ucBjneGoCv73tNww4HBu2vqUNjDpLNne7yGRYbDfz.

The freespace metric thing doesn't make any sense to me.  Here's ipfs-cluster-0
# ipfs-cluster-ctl health metrics freespace
12D3KooWM1JxwDj3g7dBTtmuKvpD4xgHsaJ4Ze97R4rtF5Wm38wL | freespace: 71 GB | Expires in: 17 seconds from now
QmVR5ucBjneGoCv73tNww4HBu2vqUNjDpLNne7yGRYbDfz | freespace: 76 GB | Expires in: 26 seconds from now

And ipfs-cluster-1:
# ipfs-cluster-ctl health metrics freespace
12D3KooWM1JxwDj3g7dBTtmuKvpD4xgHsaJ4Ze97R4rtF5Wm38wL | freespace: 71 GB | Expires in: 27 seconds from now
QmVR5ucBjneGoCv73tNww4HBu2vqUNjDpLNne7yGRYbDfz | freespace: 76 GB | Expires in: 20 seconds from now

IPFS cluster may need a bit more storage(DAG?).  Not overrun but more than half is used:
/dev/sdd                  7.8G      4.8G      3.0G  62% /data/ipfs-cluster

Thanks for looking into this. Given ipfs is a foundational component of our infra it's anyway good for us to get more expertise...
One thought: are those ssd disks? I remember for the Dapp prod cluster we moved from spin to ssd to deal with performance issues.

One thought: are those ssd disks? I remember for the Dapp prod cluster we moved from spin to ssd to deal with performance issues.

You asked that last week.  They are.

General resource stats and load stats don't really seem different when it's running slow.  IPFS is using 11-15GB of memory, presumably caching.  I haven't checked this yet during fast times so i don't have a comparison.  It's borderline right now.  Sometimes times-out by seconds, sometimes deploys in time.
2020-07-14T20:11:36.005Z [INFO]  origin-ipfs-proxy: /add?name=origin-test&recursive=true&stream-channels=true
2020-07-14T20:12:15.894Z    INFO    cluster ipfs-cluster/cluster.go:1416    pinning QmRhVUTzwpTmKj3SoH4aUBfdpeKM7JEix7VTLGA76r7JS1 on [12D3KooWM1JxwDj3g7dBTtmuKvpD4xgHsaJ4Ze97R4rtF5Wm38wL QmVR5ucBjneGoCv73tNww4HBu2vqUNjDpLNne7yGRYbDfz]:
2020-07-14T20:12:15.974Z    INFO    crdt    crdt/consensus.go:210   new pin added: QmRhVUTzwpTmKj3SoH4aUBfdpeKM7JEix7VTLGA76r7JS1
2020-07-14T20:12:15.977Z    INFO    crdt    go-ds-crdt@v0.1.12/heads.go:82  replacing DAG head: bafybeifsorapbvcfuwfv27j7v5fcchz3qxt2zn6pmdibiobfpeopabkzfq -> QmNh79CLw4LuEXizVdGpXHFwM1u22NLZk36j74Mh54LdR8 (new height: 322693)
2020-07-14T20:12:15.981Z    INFO    adder   adder/adder.go:182  QmRhVUTzwpTmKj3SoH4aUBfdpeKM7JEix7VTLGA76r7JS1 successfully added to cluster
2020-07-14T20:12:15.981Z    INFO    restapilog  rest/restapi.go:113 <IP_ADDRESS> - - [14/Jul/2020:20:11:36 +0000] "POST /add?name=origin-test&recursive=true&stream-channels=true HTTP/1.1" 200 15173

2020-07-14T20:12:17.254Z    INFO    ipfshttp    ipfshttp/ipfshttp.go:378    IPFS Pin request succeeded: QmRhVUTzwpTmKj3SoH4aUBfdpeKM7JEix7VTLGA76r7JS1

This actually might be fixed now.  The cluster is configured on rinkeby/staging again. And my tests on prod(separate from our backend deployment) have looked good so far.
Guardedly optimistic. If I don't hear any complaints today about staging I'll configure prod, document the change here, and wrap this up.

Not entirely fixed.  Down to ~40s, but there's still considerable delay happening here.  Not ready to close just yet.

Comfortable closing this one for the time being unless it shows up again.

Re-opening per our discussion during the weekly meeting today.

Closing because IPFS isn't our primary file host any longer or pinner and any pinning slowness is kind of being solved by async deployment work going on right now.


Questions about the constructor parameters of the ApriltagBoard class

hello.
I'm using this awesome library, but I'm having some problems and would like to ask you about them.
First I want to ask you about the constructor of the ApriltagBoard class, where I have a few arguments that I don't quite understand. I searched the documents you provided, but I found that they did not introduce their meanings in detail, such as scale and size. Can you explain it to me?
I always can't determine the specific size when printing (the tag printed out every time is too big), I don't know whether these parameters are not set correctly, or because there is a mistake in the setting when I print.
PS: It feels good to provide more detailed documentation or function comments so that others using the library can easily understand its usage.
In addition, I want to create "tagstanded41h12" family of apriltags, found that there is no way to generate the board, I open the source code and find that the code about apriltags v3 has been commented out, is there any reason for this? I made some simple changes and found that the code that was commented out worked fine.
Finally, I apologize in advance if I was unclear. My English is not very good. I wrote this issue with translation software. I hope to get your reply soon
With best wishes

hello ... sorry for the delay. I have been really busy, give me a few more days to get back to you on this. I wrote this library a while ago and haven't used it in while so I have to review the code when I get some free time.

Glad to hear from you
Thank you very much for your willingness to spend your rest time to help me answer my questions. I am looking forward to your reply

If you look at the jupyter notebook it shows how to create a board. This results in an image (1520x2240 pixels in the notebook example). When you print out the board, it is like printing an image and your printer preferences will probably scale the image to the whole sheet of paper.  Then measure the printed board tags with a ruler for calibration.
Maybe I could have offered a PDF output so you could define in python you want the tags to be 2 cm on each side, but that was more work.
So this shows you how to generate 41h12 tags which is a gen 3 tags. However, you want the tag to have a solid black boarder around the tag but that doesn't happen with gen 3 tags ... see the mosaic of 41h12 tags towards the bottom of the notebook or a screenshot shown below.

See how some of the tags don't have black at the corners, that won't work for calibration.
Compare that with the calibration board here. The calibration algorithm looks for the nice chessboard intersections and you don't get that with gen 3. Here is the OpenCV camera calibration tutorial.

See how the calibration algorithm detects the intersection of the squares.
So if you look at my code here:
https://github.com/MomsFriendlyRobotCompany/moms_apriltag/blob/396de2a308daa59c1c8fd3eaf160961a6dcfa736/moms_apriltag/target.py#L86
I commented out using gen 3 tags because they didn't easily work and gen 2 was good enough.
I would suggest using gen 2 tags for your calibration board. Otherwise you can create your own board using the example mosaic code in the notebook I suggested above and maybe write a new calibration algorithm.
Good luck!


refactor: update SecretInput component to support password type and add eye icon

Description
Linear Ticket
https://linear.app/autokitteh/issue/UI-807/webui-password-input-eye-icon
What type of PR is this? (check all applicable)

[ ] 💡 (feat) - A new feature (non-breaking change which adds functionality)
[ ] 🔄 (refactor) - Code Refactoring - A code change that neither fixes a bug nor adds a feature
[ ] 🐞 (fix) - Bug Fix (non-breaking change which fixes an issue)
[ ] 🏎 (perf) - Optimization
[ ] 📄 (docs) - Documentation - Documentation only changes
[ ] 📄 (test) - Tests - Adding missing tests or correcting existing tests
[ ] 🎨 (style) - Styles - Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
[ ] ⚙ (ci) - Continuous Integrations - Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
[ ] ☑ (chore) - Chores - Other changes that don't modify src or test files
[ ] ↩ (revert) - Reverts - Reverts a previous commit(s).

Ask @haimzlato if we want it as a password input and in the add form.


ddclient does not build/install

Issue description
ddclient fails to build/install.
Steps to reproduce
/etc/nixos/configuration.nix:
services.ddclient.enable = true;
Technical details

System: 16.09pre82105.7465bcd (Flounder)
Nix version: nix-env (Nix) 1.11.2
Nixpkgs version: "16.09pre82105.7465bcd"

At least the build is fixed on master.

ddclient builds OK, and services.ddclient.enable = true enables the service (and the services doesn't enter "failed" state) on both master (b11186a6369106d712149522c8a2f18ca5811cf6) and release-16.03 (0f9e365e0f7d3c12626d3e733d87a28a12567b71).
Please re-open of the issue appears again.

Seems to be fixed. Thanks.


Fixing InPlayEval

I'm pretty sure the reason we were seeing Kenobi get confused about when the ball was in play this weekend was because InPlayEval was capturing the ball's starting position at the wrong time. So, if you entered the kickoff prep command then moved the ball more than 0.5m before issuing normal start, the ball would immediately be considered in play.
This PR rewrites InPlayEval to hopefully follow the rules correctly. Eventually we should add real tests, but based on my informal testing in simulation, this works correctly and doesn't get tripped up by things like the ball moving during prep commands.

I'm okay with this for now and think the logic makes sense, but I think we need to make actual tests for this before comp to verify.

Agreed on having tests. Pretty much everything in Kenobi would benefit from tests.


Support SELECT .. FOR NO KEY UPDATE / KEY SHARE row level locking clauses in Postgres

In addition to SELECT .. FOR UPDATE and SELECT .. FOR SHARE, Postgres supports FOR NO KEY UPDATE and FOR KEY SHARE row level locking clauses https://www.postgresql.org/docs/14/explicit-locking.html#LOCKING-ROWS.
This adds support as .forNoKeyUpdate() and forKeyShare().
I don't know if there's an equivalent feature in MySQL and MSSQL - how should that be handled?
Fixes #4754

Looks good! Could you please also submit documentation PR to https://github.com/knex/documentation?


Gmail happens the send multiple FETCH response for a same seq/uid

I cannot find the original context (I am pretty sure I already faced this issue in the past), but this is the most recent issue I have: https://github.com/pimalaya/himalaya/issues/498.
To give a bit more context, this happens when reading an unseen message. The first FETCH contains Uid(n), Flags([Flag(Seen)]), BodyExt, and the second contains Uid(n), Flags([Flag(Seen)]).

Question: Does imap-client emitted a warning in this case? Just curious.
As IMAP is a cache-filling protocol, it seems to be a valid thing to do. The client is expected to update it's state twice in this case. For imap-clients command-based interface I think we should merge duplicate responses. What do you think?

Does imap-client emitted a warning in this case?

Yes it does: https://github.com/pimalaya/imap-client/commit/39d0217a58637a5f7e017e12bd8845f562bfd5b6#diff-b0a0498b0d99e8443353139a00701d7471af24e5567dda0a4e9ee7e5f23e496eR62

I think it is. Because: If you look at IMAP as "updating your database" constantly protocol, sending data in pieces would have the same result as sending at once. But let's maybe keep this issue open in case someone else has a more clear answer.


Drop Search Results with potentially unsafe location values

Introduces filtering of unsafe search result locations.
The location value of the document is used directly by the FE. Essentially, it's possible to execute JS in the browser in the context of a logged-in user by modifying a search result in the DB to use the javascript: pseudo protocol (e.g. javascript:alert(/xss/)). This assumes the DB is compromised so this is really damage limitation. It was also highlighted to me that a naive Collator might have an exploit allowing a user to inject something like that but that's hypothetical.
Signed-off-by: Iain Billett<EMAIL_ADDRESS>
[x] A changeset describing the change and affected packages. (more info)
[ ] Added or updated documentation
[x] Tests for new functionality and regression tests for bug fixes
[ ] Screenshots attached (for UI changes)
[x] All your commits have a Signed-off-by line in the message. (more info)

Hi @iain-b! What's the motivation for this change? We're currently working on integrating the new permissions framework into search, and I'm wondering whether using permissions to restrict access to search results at query time could provide a more general-purpose solution to this problem.
If these results should be omitted for all search queries, I think @camilaibs makes a great suggestion above. One big benefit of filtering at index-time is that it avoids returning a reduced number of results for a given page when results are filtered out.

Hi @iain-b! What's the motivation for this change? We're currently working on integrating the new permissions framework into search, and I'm wondering whether using permissions to restrict access to search results at query time could provide a more general-purpose solution to this problem.
If these results should be omitted for all search queries, I think @camilaibs makes a great suggestion above. One big benefit of filtering at index-time is that it avoids returning a reduced number of results for a given page when results are filtered out.

@mtlewis This actually comes from a security review which considered the impact of the DB being compromised. This highlighted that the location value of the document is used directly by the FE. Essentially, it's possible to execute JS in the browser in the context of a logged-in user by modifying a search result in the DB to use the javascript: pseudo protocol (e.g. javascript:alert(/xss/)). It was also highlighted to me that a naive Collator might have an exploit allowing a user to inject something like that but that's hypothetical.
Filtering wasn't really my intention but that seemed like the safest thing to do in this instance (and gives a better user experience than a search result without a location). I'm happy to simply scrub the location value rather than dropping the whole search result?

@iain-b super helpful context, thanks! This doesn't seem like a use case that the permissions framework is relevant for. I do still think it's worth considering @camilaibs's idea to filter at index time inside the IndexBuilder - that way the unsafe documents never get into the index, and as a bonus, filtered-out documents won't result in fewer than  documents coming back for a given query.

to filter at index time inside the IndexBuilder - that way the unsafe documents never get into the index

That is an interesting idea. It seems like an nicer point in the application to do this but our original cause to look at this was from the point of view of DB access being compromised. In other words the search result is changed manually after it is indexed. I understand this is very defensive. The filtering could be done at both points?

That makes a lot of sense! I'm getting a new perspective on this change - I see now that the filtering is handling an unexpected situation in the app, which means keeping pagination predictable doesn't really need to be a priority. Worth considering doing it in both places, but I do see the sense in just doing it at query time too!


Feat/stories

Add template for pull requests

Sorry for the delay. Wanted to solve problems on my branch first.
Approved


feat: move kubernetes dir into templates to support jinja2 across all files

This PR moves the kubernetes dir into the templates directory and renames the files to .yaml.j2. This is being done to support templating across all the files in the kubernetes directory and will give us the ability to make this more modular.

@mrueg if you have a minute I'm looking for feedback on if this a good idea or not.

I believe this is a good enough first pass, most file changes here are just a rename as the root kubernetes folder doesn't exist anymore.

@onedr0p I like the idea, it might be a bit more difficult for folks to grasp, as it's adding another layer of abstraction though

I think overall it will work out, I just need to add more documentation around it
It was nice adding support for this https://github.com/onedr0p/flux-cluster-template/pull/837


chore: improve next steps message

Checklist

[ ] I have added changeset via pnpm run change.
[x] I have updated the documentation.
[x] I have added tests to cover my changes.

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

Nice catch! please sign the CLA ⬆ 😄

👏👏


Added terrain material with bump mapping support to materials library

Based on a mixmap (RGB)
Support of bump mapping
Documentation is coming

This is excellent!

Doc is required as you mentioned :)

I'd like to add some textures to the playground and create a better Mixmap to create an awesome playground example in the documentation

Feel free to send me those textures


Change the Breadcrumb names into one  line on mobile view

Purpose

The purpose of this PR is to fix #177
Goals

The breadcrumb should display all names on a single line

Approach

Change text-sm  to text-xs sm:text-sm md:text-base that the text size will be xs (extra small) on mobile screens, change to sm (small) on small screens (640px and above), and increase to base (normal size) on medium screens (768px and above).

Screenshots

Checklist

[x] This PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.
[x] I have read and understood the development best practices guidelines ( http://bit.ly/sef-best-practices )
[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[ ] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation

Related PRs

Test environment

Learning

Good job! @Sanjana0823

Thank you @anjula-sack


FR: Allow snapshot listeners to be paused

Feature proposal

Firebase Component: Firestore

Problem: To stop a snapshot listener from performing when the client is in the background, the listener must be removed. To resume listening, the listener must be re-initialized with a new closure. However, extra steps must be taken to preserve references to objects that have subscribed their completion handlers to the original closure.
Solution: Allow snapshot listeners to be paused and resumed (individually and/or collectively).

Hi! Thanks for the feature request.
Our API is already pretty large and while I can appreciate this could make some tasks easier, it seems like it's straightforward enough to capture your closure separately if you want to:
let listener = { snap, error in ... }
var registration = db.addSnapshotListener(listener)

// pause your listener
registration.remove()

// resume listening
registration = db.addSnapshotListener(listener)

Also note that it's already possible to effectively pause all listeners. If you call disableNetwork, Firestore will shut down all of its listeners (but also pause sending any outbound writes).


大佬  能帮看一下哪里出错了吗，用docker部署

[root@VM-20-6-centos Meco]# docker run -d -p 80:80 --rm vuepress-docker --name meco
Unable to find image 'vuepress-docker:latest' locally
docker: Error response from daemon: pull access denied for vuepress-docker, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.
See 'docker run --help'.

附图


Add dockerfile for CI build

As per as the ML discussion, this adds the Dockerfile to build the project. It also adds instructions on how to use it.

SUCCESS
4927 tests run, 0 skipped, 0 failed.
--none--


Made the product name consistent with other docs.

#sign-off

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

@ranvijaykumar You are using the public repo. Be sure to use azure-docs-pr for your edits.

#sign-off


Kafka pubsub configuration require consumerID

Describe the issue
Kafka pubsub configuration missing consumerID in spec.metadata which used as ConsumerGroup, If not specify it, will cause
FATA[0005] process component pubsub error: init timeout for component pubsub exceeded after 5s in startup log and daprd process will be terminated
URL of the docs
setup-apache-kafka
Expected content
apiVersion: dapr.io/v1alpha1
kind: Component
metadata:
  name: kafka-pubsub
  namespace: default
spec:
  type: pubsub.kafka
  version: v1
  metadata:
      # Kafka broker connection setting
    - name: brokers
      value: "dapr-kafka.myapp.svc.cluster.local:9092"
+   - name: consumerID
+     value: "consumerIDVal"
    - name: authRequired
      value: "true"
    - name: saslUsername
      value: "adminuser"
    - name: saslPassword
      value: "KeFg23!"
    - name: maxMessageBytes
      value: 1024

Screenshots

Additional context

Found that can initialize initTimeout: 1m to solve this problem


Ensure helm issues 1776 and 1779 are addressed

See https://github.com/kubernetes/helm/commit/d762a42168308d93a46be1d7bc37bc518eee496f; our .helmignore parsing logic probably already handles this properly.

See also https://github.com/kubernetes/helm/issues/1776 and https://github.com/kubernetes/helm/issues/1779.


check_failure fails to fail

Prerequisites

[x] Put an X between the brackets on this line if you have done all of the following:

Checked that your issue isn't already filed.
Reduced the issue to a self-contained, reproducible test case.

Description
From type_classes Output Parameters
By default, Lean only tries to synthesize an instance Inhabited T when the term T is known and does not
contain missing parts. The following command produces the error
"failed to create type class instance for Inhabited (Nat × ?m.1499)" because the type has a missing part (i.e., the _).
#check_failure (inferInstance : Inhabited (Nat × _))

Instead  #check succeeds printing :
inferInstance : Inhabited (Nat × ?m.5)

And the same for this example from line 368
-- Error "failed to create type class instance for HMul Int ?m.1767 (?m.1797 x)"
#check_failure fun y => xs.map (fun x => hMul x y)

#check says:
fun y => List.map (fun x => hMulx x y) xs : (y : ?m.79) → List (?m.109 y)

Steps to Reproduce
See above
Expected behavior: [What you expect to happen]
failure
Actual behavior: [What actually happens]
no failure, see CI failure:
https://github.com/leanprover/theorem_proving_in_lean4/runs/4193469732?check_suite_focus=true
Reproduces how often: [What percentage of the time does it reproduce?]
Versions
You can get this information from copy and pasting the output of lean --version,
please include the OS and what version of the OS you're running.
Additional Information
Any additional information, configuration or data that might be necessary to reproduce the issue.

Whoops...

@Kha This was an intended side-effect of a recent change. A few users asked for the #check to succeed even when there are pending TC problems. They pointed out that this is the behavior in Lean 3.
The #check_failure is based on #check, and the change triggered this regression.
The issues have been fixed, and I am closing the issue here.


Persist RawEntityDescriptor and associated models

Validates parameters and persists:

A new RawEntityDescriptor
A new KnownEntity
A new EntityId

The enabled field is used from the payload when creating these models.
Updated/created timestamps in the request are no longer used. See https://gist.github.com/rianniello/8eebbefb7dbf9e894397c43edf0709e5. This means we do not store when an IdP was created (happens in Hosted IdP Service). From some exploration this shouldn't be an issue.
Right now when a RawEntityDescriptor is created the response code is 201. At this point a location header cannot be included (i.e. GET /api/entity_sources/:tag/raw_entity_descriptors/:id) as this resource doesn't exist yet — something that may be required in the future.
CC the @bradleybeddoes.

Some interesting behaviour I noticed when testing this on a real instance, I can keep persisting the same payload over and over without uniqueness constraints/failures. Unsure if this is intentional with the saml service data model (are we allowed multiple EntityIDs that are the same)?

Have added https://github.com/ausaccessfed/saml-service/issues/122 to address duplicate issue. Until then we're expecting sources to only provide unique entityID values (i.e. they have a local unique constraint as well).

Have reworked validations into models, ready for another review 👍

Moved validations into base class https://github.com/ausaccessfed/saml-service/pull/121/commits/42f1163ac1e29fdbf243a88ce07dcf3eccde0544.


fix: check if version tag exists

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

92 of 106 (86.79%)  changed or added relevant lines in 2 files are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.6%) to 88.751%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

Versionize/RepositoryExtensions.cs
11
15
73.33%

Versionize/WorkingCopy.cs
81
91
89.01%

Files with Coverage Reduction
New Missed Lines
%

Versionize/WorkingCopy.cs
2
88.56%

Totals

Change from base Build<PHONE_NUMBER>:
0.6%

Covered Lines:
773

Relevant Lines:
871

💛  - Coveralls


Add explicit links to the GCSE sections

Context
This adds in new and create actions/views/routes for all the GCSE controllers. This allows us to pass in explicit link to all the backlinks.
This ones a bit more complex than most of the others on quite a few views the backlink could go back to multiple locations based on whether its an international qual or if they input a gcse with a grade of D or worse.
Changes proposed in this pull request

Add a new and create action to each controller
Add the routes
Add the new view
Ensure backlinks have explicit values passed in

Guidance to review
Per commit is the way to go. It's 1 commit per controller
I've given it a pretty thorough manual test and it seems to be behaving, but it's worth double checking
Link to Trello card
https://trello.com/c/GuiJ30Jg/3397-catch-all-give-back-links-explicit-targets
Things to check

[x] This code does not rely on migrations in the same Pull Request
[x] If this code includes a migration adding or changing columns, it also backfills existing records for consistency
[x] API release notes have been updated if necessary
[x] This code does not rely on the addition/removal of Azure config environment variables in the same Pull Request
[x] New environment variables have been added to the Azure config

Hey Frankie, just pinging you because i know how much you love backlinks haha

Thanks for the review Steve. It looks like the second issue is prexisting, but youre right it's undesirable. Will sort Tuesday 👍
Have a great long weekend!

Thanks for all the feedback Steve.

I can't seem to replicate the error
This is a pre-existing issue. I'll card it up!


cellular response to nutrient/ cellular response to nutrient levels

anyone know why these are not related?
cellular response to nutrient/ cellular response to nutrient levels
The bigger question to me is why we need both terms?  Any insight into this?
Reported by: ValWood
Original Ticket: geneontology/ontology-requests/11413

Changed my mind after looking over usage . Keeping response to nutrient levels as general class.  This makes sense as it covers response to the absence of a nutrient (e.g. response to starvation).


implement List component

@tanbowensg Could you write down some design decisions in this PR's description? Then it would be great for reviewers to get the whole picture of this feature, including 'why and why not'.


Add premultipliedLast BitmapInfo to allow transparent background

When creating a CGImage in cgImageFromFramebuffer method in PictureOutput class, the CGBitmapInfo was empty. Creating a CGBitmaInfo with a GCImageAlphaInfo.premultipliedLast rawValue it will take care of the alpha channel.
Any drawbacks about this?

This fix worked well for me.  Note that the fix should be applied to both the iOS and Mac PictureOutput.swift:

GPUImage2/Source/Mac/PictureOutput.swift
GPUImage2/Source/iOS/PictureOutput.swift

... confused me for a while as I was testing on iOS and the changes in this PR are made for the Mac.


Make compatible with pandas 1.2.0

Some tests fail with the most recent version of pandas.

never mind, everything appears to work with pandas 1.2.3.


Explain .threads = ?, .workers = ?

I was able to use
    zap.start(.{
        .threads = 1,
        .workers = undefined,
    });

without an issue. This is mentioned in https://facil.io/0.7.x/index. What does the workers field do?

From https://facil.io/0.7.x/fio#running-facil-io

threads: The number of threads to run in the thread pool.
workers: The number of worker processes to run (in addition to a root process)

More specifically:

"If the other option (i.e. .workers when setting .threads) is zero, it will be automatically updated to reflect the option's absolute value. i.e.: if .threads == -2 and .workers == 0, than facil.io will run 2 worker processes with (cores/2) threads per process."

I agree, we should put doc comments to pub fn start(); thx, @jeroenflvr for looking it up.
But now for something more important:
        .workers = undefined,

NEVER do this!!!!
It will set .workers to 0 in debug builds if you're lucky, but is likely undefined in release. Meaning, one time it might start 123 workers and the next time 250.
undefined in Zig means something different than you may assume. It does not mean: "I don't want to define this, so please take a default value."
If that's what you wanted to express, it would be: .{ .threads = 1 }: omitting .workers, so Zig can take the default value.
In Zig, undefined is NOT a value. You cannot check if anything is undefined, like: "if it's undefined, then set it to 5".
undefined means: just reserve memory for a variable, but DON'T write to it, don't initialize it. I promise, I will initialize it later:
var x : u8 = undefined;

// x can have any value now, from 0 to 255

// ... do some other stuff that does not involve reading from x ...

x = 1;

// now x has a defined value, we kept our promise to initialize it later

Above example is probably more easy to understand. Now imagine this:

fn foo(struct{ a: u8, b: u8}) void {
    std.log.debug("a={d}, b={d}", .{a, b});
}

test foo {
    // now call foo with **random, uninitialized garbage**
    foo(.{ .a = undefined, .b = undefined });
}

That is not what undefined was made for. You basically pass garbage for a and b if you do that.
// now with default values
fn foo(struct{ a: u8 = 0, b: u8 = 0}) void {
    std.log.debug("a={d}, b={d}", .{a, b});
}

test foo {
    foo(.{                 .b = 3});   // OK: a will take the default
    foo(.{ .a = undefined, .b = 3});   // CRAP: a will be "random garbage"
}

Running facil.io
The facil.io IO reactor can be started in single-threaded, multi-threaded, forked (multi-process) and hybrid (forked + multi-threads) modes.
In cluster mode (when running more than a single process), a crashed worker process will be automatically re-spawned and "hot restart" is enabled (using the USR1 signal).

I assume that using process is better than threads (on Linux it's all process with or without memory sharing).

General assumptions are always bad. I do not agree that "using process" is better than threads, in general. "better" in terms of what?
If you use more than one worker process, you'll have to come up with IPC to "share state" between processes.
In complex "apps', I always used 1 worker with n threads, to get that auto-mem-sharing. I favored using sth like a hashmap in RAM over using some form of persistence just to be able to access data from multiple workers.
If all you do is serving static content or even files, so all requests are independent of each other, then using multiple worker processes is an option. But not necessarily the better option. I would strongly advise taking some actual, accurate, measurements or observations before assuming.

This is off-topic, but I'm willing to trade memory for reliability.

I agree it's off-topic. But now it seems you suggest processes provide higher reliability than threads? In the context of facil.io / zap? Do you know that? Or do you assume that? What do you even mean with "reliability"? (These are all rhetorical questions, I don't want to go into a debate. Especially here, and I don't have time for it anyway. :smile:).
I just don't want to leave such statements unchallenged here. Especially with other newbies reading them, giving the topic of the issue.
I just don't agree. I am not arguing for threads. I am arguing against such condensed pieces of wisdom that leave out too much to be applicable in general.


Minor update to downloader logs

Display full paths instead of just names so we know what is processed in case of multiple releases.

I also want to test new Zuul hook with this pull request :-)


Private Messages: Scroll the left-sidebar container to desired pm whe…

…n not in view.
Fixes #23609: scroll selected pm when not in view by using the scroll_util module to scroll the left-sidebar-container inside pm_list.js which is used in a similar fashion in stream_list.js.

Fixes: #23609: Scroll selected pm when not in view.
Major changes:

Imported the scroll_util module inside pm_list.js.
Created two new functions inside pm_list.js to increase code reuse - get_user_id() and get_user_li().

Screenshots and screen captures:
Screencasts showing the differences are attached below:

Above is the original behavior

Above is the new behavior after code changes.

Self-review checklist

[x] Self-reviewed the changes for clarity and maintainability
(variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

[x] Explains differences from previous plans (e.g., issue description).
[x] Highlights technical choices and bugs encountered.
[x] Calls out remaining decisions and concerns.
[x] Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

[x] Each commit is a coherent idea.
[x] Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

[x] Visual appearance of the changes.
[x] Responsiveness and internationalization.
[x] Strings and tooltips.
[x] End-to-end functionality of buttons, interactions and flows.
[x] Corner cases, error conditions, and easily imagined bugs.

Works for me in manual testing, using the p reproducer in the issue description, as well as clicking on a search results.
@amanagr Could you please review this one?

LGTM!

@timabbott This PR has been reviewed by @amanagr and me -- would be great to get it integrated!

Sorry I didn't realize that I made this PR from my main branch. I am now using new branches for my other issues.

I think this will be a very meaningful fix; so I took the time to finish the PR.
I ended up needing to rewrite the change more or less completely -- only scroll_pm_into_view is mostly unmodified. Some notes:

Removed export on functions not used in other modules (or appropriate for such use).
Removed the tiny get_user_id and get_user_li helpers that were poorly named given the specificity of what they were doing, and only 1-2 lines of code if  inlined.
Fixed this not working for the "All private messages" link. I was unable to readily debug why calling scroll_pm_into_view  with  $("#private_messages_section_header") as the parameter didn't work; perhaps if you can debug that @Ilikepizza2 it could be a nice follow-up to remove the semi-duplicate function I added to support this.

(Your work was still helpful in figuring out where the changes needed to be, and having a working version to start with, though!).
Marked to merge once CI passes with this version; thanks @Ilikepizza2 for the contribution!

Marked as a backport candidate; we'll want to have this run on chat.zulip.org for a couple weeks before potentially backporting, but I think this is a meaningful enough bug fix to the new private messages section to be worth consideration for backporting.

@Ilikepizza2 can you look into addressing the test failure?

@amanagr would you be able to rebase this and fix the text failure? It's a pretty important fix and I don't want it to get lost.

Okay

updated!


added _event to prototype

removed checks for existence of _events prop and added it to the prototype.

You will need to update the docs, as this makes it 18 lines of code.

Adding it to the prototype would cause problems in some cases.
function Test() {}
Test.mixin(MicroEvent);

var testObject = new Test();

function Test2() {}
Test2.prototype = Test.prototype;

var testObject2 = new Test2();

In this case, both testObject1 and testObject2 share the same _events object. So if testObject 2 triggers an event, any events that testObject1 set would also be triggered.

Can you write a real failing test that proves your point?

I was just saying that each object should have its own _events object regardless of how far down a prototype chain it is.

As Kevin: @kevinmctigue pointed out, this does not work correctly when object prototypes are combined via mixin.