nixos/obs-studio: init

Description of changes
Add the programs.obs-studio module.
It lets you add third-party plugins (e.g. pkgs.obs-studio-plugins.wlrobs) to make a wrapped OBS package.
This is basically a port of the equivalent home-manager module.
cc @eclairevoyant @jb55 @MP2E @materusPL @fpletz
Things done

Built on platform(s)

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

For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)

[ ] sandbox = relaxed
[ ] sandbox = true

[ ] Tested, as applicable:

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

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

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[x] Fits CONTRIBUTING.md.

Add a :+1: reaction to pull requests you find important.

I am not sure on how to test such module. I have created a test file but for now, it doesn't do much (enables the module with one extra plugin).
I don't know whether I should launch OBS in the VM and check that it was succesful.

OBS seems to have pretty good logging. It might be possible to create a test that starts OBS from some sort of external program that captures the output, waits for a cue in the log to shut it down and look for a string saying that the plugin has been loaded.

OBS seems to have pretty good logging. It might be possible to create a test that starts OBS from some sort of external program that captures the output, waits for a cue in the log to shut it down and look for a string saying that the plugin has been loaded.

Indeed, that would be nice. However, I have no clue in how to write that... I guess that I would have to import the x11 stuff in the test derivation.
Would you know any similar example that I could get inspiration from ?

Thanks for the contribution. Looking at the code here, I'm not entirely convinced the module is worthwhile atm - usually I would expect a module to enable everything required for its package to work and possibly expose some config options; I don't see either here.
Maybe wrapOBS should just be publicised/documented?

Thanks for the contribution. Looking at the code here, I'm not entirely convinced the module is worthwhile atm - usually I would expect a module to enable everything required for its package to work and possibly expose some config options; I don't see either here.
Maybe wrapOBS should just be publicised/documented?

This is a fair opinion. I could understand if this patch would get refused.
Any opinion @jb55, @fpletz, @MP2E, @materusPL, @SuperSandro2000 ?

Maybe wrapOBS should just be publicised/documented?

Where would that be? wrapOBS is not even available on https://search.nixos.org/packages. And if it were, the parameters wouldn't be discoverable. IMHO wrapOBS is not easily discoverable since its name doesn't start with obs but that is a separate issue.
Since the standard NixOS way for configuring wrappers is a NixOS module which is also done for other programs, I don't see any reason not to add this module.
The test in its current form is kind of useless, though. I would prefer to remove it and or add something useful like loading a module and starting a stream.

Where would that be? wrapOBS is not even available on https://search.nixos.org/packages. And if it were, the parameters wouldn't be discoverable. IMHO wrapOBS is not easily discoverable since its name doesn't start with obs but that is a separate issue.
Since the standard NixOS way for configuring wrappers is a NixOS module which is also done for other programs, I don't see any reason not to add this module.

I agree. This is why I drafted this implementation.

The test in its current form is kind of useless, though. I would prefer to remove it and or add something useful like loading a module and starting a stream.

Indeed ! Could you link me to some resource on how to implement such a test ? I was not able to find examples in the repo...

Indeed ! Could you link me to some resource on how to implement such a test ? I was not able to find examples in the repo...

I see the following challenges and possible solutions without having tried it:

We need a graphical environment. This is easy and there are lots of examples in the tests, just grep for tests that include ./common/x11.nix.
OBS needs to be configured. You could just put some configs for scenes and profiles in the test users home directory for this. As far as I can see these are inis, so this should be quite easy. Not sure if obs can be called with a specific config file or directory.
Starting streaming automatically could be accomplished by starting obs with --startstreaming.
You need an endpoint to stream to and check if the stream is being received. I'm currently very fond of mediamtx which I am hosting for restreaming purposes. There is a NixOS test for mediamtx you can look into. You could also use the nginx-rtmp-module as an alternative.

If you need more assistance you can also message me on matrix.

Converting to draft pending tests

Should this be closed now that #312112 is merged?

Should this be closed now that #312112 is merged?

Indeed ! I am happy that someone was able to get this through.


List Camtrap Dp resources in preferred order

In https://rs.gbif.org/sandbox/experimental/schemas.json the Camtrap DP schemas are listed as:
deployments
media
media-observations
event-observations

That is the preferred order.
In the IPT (dropdown when mapping) and the published datapackage.json, the resources are listed as:
event-observations
media-observations
deployments
media

Can the preferred order by maintained?

@peterdesmet Thank you
Why is it important please?

That order is the logical flow from the data model:

So it will be more convenient for publisher to map it as such and for consumers to see the resources listed as such.

To make it work at ipt3.gbif-uat.org we need to re-install (delete and install) Camtrap schema.


ComponentWillReact stops firing

Just updated to react 16.8. Trace always shows component update due to reactive props. The first time it fires, but on future prop updates ComponentWillReact only fires for state changes, not prop updates. Am I missing something?

Sorry using mobx 5@latest

Let me clarify. After initial fire, ComponentWillReact only gets fire by domain state changes, not prop updates.

Could you demonstrate the issue in code sandbox?
Op za 9 feb. 2019 20:12 schreef stevenfuzz<EMAIL_ADDRESS>
Let me clarify. After initial fire, ComponentWillReact only gets fire by
domain state changes, not prop updates.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx-react/issues/643#issuecomment-462071145,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABvGhMLEQ_HA1sVH8HR9buvOmJU46aCrks5vLx2KgaJpZM4aypTo
.

Yes, I'll see what I can do. So it should fire with all prop updates, right?
I haven't tested it in isolation. I use mobx in a framework I'm writing. So, it took a while to figure out why so many things were broken.
I'll try to duplicate in a simple component first.

So I created a simple component, but couldn't duplicate the issue. That being said, I am still seeing it happen. Basically (in one of several examples) several changes to the ui domain occur concurrently (set menu open state, set theme). Trace shows the reactions are observed (reactive props). ComponentWillReact fires and has the state of the first change, but doesn't fire again.
I can trace the changes that are not being fired with ComponentWillReact in ComponentDidUpdate (by keeping a variable hasReacted), and can see that mobx has called forceRender.
ComponentWillReact should always be called right? Is it possible I have a method set as an action when it shouldn't be?
I guess I could regress react to an older version, try to figure out where things stopped working. What can I do to help? Would the trace of the prop change that didn't fire ComponentWillReact help?

Ok. Figured it out. In upgrading, the peer dependencies for mobx got messed up between the app and framework. I didn't realize this until I reverted back mobx and got a warning. I wish I saw this in the latest version...


A Frame light not working if CanvasWidth and CanvasHeight specified

Describe the bug
I had issue setting up lights in my project until I randomly experimented with the CanvasWidth and CanvasHeight parameters. If they are not defined, ambient or hemispherical lights do not work.
To Reproduce
Steps to reproduce the behavior:

Create a simple setup that loads a model 3d model (important, a frame's primitives do not get shaded).
Add an hemispherical/ambient light and a point light to the scene inside the a-marker
Run the project on Chrome-Android, you'll see the model is dark.
Go back to your project and in the arjs parameters set canvasWidth: 1920; canvasHeight: 1080; (assumming you are runnning landscape).
Voila, now the scene is properly lit.

Expected behavior
Lights should work regardless of canvasWidth and CanvasHeight definition. They work in a-frame without arjs or in a-frame Inspector Mode (desktop),

Closing this issue due to dismission of this repository. New repository of AR.js will be up at https://github.com/AR-js-org/AR.js/
Feel free to re-open the issue there if still valid.


Questions about 328P

Hi,
I'd like know if this lib can be used with platformio without the arduino framework. Probably I need to program directly the ic to get full control and avoid using middle frameworks.
Thanks
R

good question. Have never tried this.
If you can provide a minimal platformio project without arduino framework, I can support to make it compatible.


Don't log api errors with log level set to zero

API errors can be quite spammy if some service is requesting pod metrics and the cluster has lots of short-lived pods, as can be when using ScheduledJobs. Even if heapster is started with --v=0.
This change makes these messages to require at least a log level of 2 to be logged.

This change is

/lgtm

This seems reasonable.  We commonly use these errors to diagnose issues with Heapster, but V(2) is sufficiently low that it's probably fine to just run with that when we care about it.


Broken Links in Wiki

On page https://github.com/radiant/radiant/wiki
When we click on Installation, The Basics , extensions and other stuff it redirects us to https://github.com/radiant/radiant/wiki#wiki-extensions..
And it should be https://github.com/radiant/radiant/wiki/extensions..
And its does not work for the other links too
please update asap
thanks!

it's a wiki for a reason...


implement imports.exclude

At present cargo-vet largely punts on conflict handling, we should define it.
Generally speaking, audits are additive and can't generate conflicts. However, it can happen if the range provided in a violation entry intersects a leaf in the audit version graph. This basically means that two parties explicitly disagree on whether the criteria are met for that version. Examples might include:

Somebody missed a problem when certifying an audit.
Somebody provided an overly-broad violation range.
Two parties fundamentally disagree on whether there's a problem.

I think (1) and (2) are the most likely cases, and so I think we want the default behavior to result in a discussion and ideally a consensus resolution. However, we can't have the tool entirely fall apart while that discussion is happening, or in the actual case of (3). So we need some mechanism to immediately resolve the conflict locally.
My spitball suggestion here is to add an exclude field to imports table entries, which lists a set of crates for which audits are dropped for that import. That strikes me as a lightweight mechanism to resolve conflicts in either direction (local violation + foreign certification, or vice versa), while also ensuring that any such conflict is surfaced for manual inspection.
@tmandry wdyt?

I think I've been picturing a local override more as a flag only local audits can have with an extra flag for "and i mean it", ala !important in CSS, but maybe that's overly broad?
The flipside over-engineered version of this would be generating a uuid for each forbid entry so people can explicitly name it.

Yeah, my thinking was that being able to selectively reject imported audits as proposed above is a simple safety valve that doesn't complicate the model or lead to surprising results.

One case where I could see fundamental disagreements cropping up are where we have unsafe code that is technically undefined behavior but also unlikely to ever be a problem. We come across this fairly often. (recent example)
In any case:
A problem I see with the model being proposed here is that a trusted upstream audit source can suddenly cause cargo vet to start failing without any changes, which is not nice if it's running in CI. Off the top of my head there are a few approaches I see that could work...

Warn when there's a conflict, and ask the user to resolve it somehow to remove the warning, but don't fail
Vendor upstream audits with a command that updates them periodically. These vendored files then become the source of truth until they are updated again.

Yes there is already a "lockfile" for foreign audits -- imports.lock, one of the 3 files managed by cargo vet and which is already minimally implemented. It's just a cache of all the foreign files, nothing fancy.
The expected workflow is that CI will run with cargo vet --locked and therefore not update the lockfile at all. That said, because the stakes are a lot lower we currently always try to update the lockfile when cargo vet is run without --locked, unlike Cargo.lock, which tries to stay static more aggressively. This is presumably a good thing because you do want to know if one of your cohorts notices a serious issue and forbids something. It's just a question of what you do once you learn about that.

I added exclude to the book, morphing this issue to be about implementing.

I think this will be slightly easier to implement in a clean way once we've also fixed #238, as that way we can more easily track which source each imported entry comes from and do these exclusions locally after the lock has been updated.
I imagine we'll want to have the excluded entries downloaded and in the lockfile to keep formatting and error reporting anyway.


Link to a sample Web Application

Design looks good, but a sample working Web Application will help a long way in understanding the implementation aspects.

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

ID: dbb49e16-b543-30ae-88e5-2241836e2510
Version Independent ID: d42b1eab-9682-ea69-f203-e519b456a56f
Content: ASP.NET Core Data Protection
Content Source: aspnetcore/security/data-protection/introduction.md
Product: aspnet-core
GitHub Login: @Rick-Anderson
Microsoft Alias: riande

@blowdart can you provide a link to a sample app?

Not really, it's all done by default. This isn't really meant as a topic for samples, but rather what you go to for reference. Data Protection is wired up automatically by aspnet


how to modify the Request?

What versions are you running?

$ go list -m github.com/chromedp/chromedp
github.com/chromedp/chromedp v0.6.10
$ Chrome --version
90.0.4430.85
$ go version
go version go1.15.6 windows/amd64

What did you do? Include clear steps.
handle the fetch.EventRequestPaused and modify the Request
the code:
c := chromedp.FromContext(ctx)
ctx := cdp.WithExecutor(ctx, c.Target)
if ev.Request.URL == "http://testphp.vulnweb.com/search.php?test=query" {
	headers := make([]*fetch.HeaderEntry, 0)
	headers = append(headers, &fetch.HeaderEntry{Name: "Content-Length", Value: "26"})
	headers = append(headers, &fetch.HeaderEntry{Name: "Content-Type", Value: "application/x-www-form-urlencoded"})
	fetch.ContinueRequest(ev.RequestID).WithMethod("POST").
		WithHeaders(headers).
		WithPostData(`searchFor=dddd&goButton=go`).Do(ctx)
}

What did you expect to see?
i hope to modify the Request:
POST /search.php?test=query HTTP/1.1
Host: testphp.vulnweb.com
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:88.0) Gecko/20100101 Firefox/88.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
Accept-Language: zh-CN,zh;q=0.8,zh-TW;q=0.7,zh-HK;q=0.5,en-US;q=0.3,en;q=0.2
Accept-Encoding: gzip, deflate
Content-Type: application/x-www-form-urlencoded
Content-Length: 26
Origin: http://testphp.vulnweb.com
Connection: close
Referer: http://testphp.vulnweb.com/search.php?test=query
Upgrade-Insecure-Requests: 1

searchFor=dddd&goButton=go

What did you see instead?
the method is modified, but modify the headers unsuccessfuly
the follow:( get the request by BurpSuite)
POST /search.php?test=query HTTP/1.1
Host: testphp.vulnweb.com
Content-Length: 0
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.103 Safari/537.36
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9
Accept-Encoding: gzip, deflate
Accept-Language: zh-CN,zh;q=0.9
Connection: close

I have personally not use the fetch domain much. A quick reading of the fetch protocol code indicates that the fetch domain needs to be enabled via a call to fetch.Enable(). I'll try to put together a working example.

I found the issue; apologies here. chromedp predates the existence of the fetch API, and what's happening is a mutex is being locked during Navigate. This can be worked around, but appreciate the understanding here in that this requires a slight modification to chromedp's internal plumbing. I'll attempt to get a fix ASAP.

thanks @kenshaw

@kenshaw  sorry ,i have an another question, how to post the request except the modify http method via fetch.EventRequestPaused

how to post the request except the modify http method via fetch.EventRequestPaused

@xxxxnnxxxx can you elaborate this?  It's not clear about what do you want to do.

@ZekeLu
i want to send a post request ,example
POST /search.php?test=query HTTP/1.1
Host: testphp.vulnweb.com
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:88.0) Gecko/20100101 Firefox/88.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
Accept-Language: zh-CN,zh;q=0.8,zh-TW;q=0.7,zh-HK;q=0.5,en-US;q=0.3,en;q=0.2
Accept-Encoding: gzip, deflate
Content-Type: application/x-www-form-urlencoded
Content-Length: 26
Origin: http://testphp.vulnweb.com
Connection: close
Referer: http://testphp.vulnweb.com/search.php?test=query
Upgrade-Insecure-Requests: 1
Myheader: "example"   <------------------------------custom header

searchFor=dddd&goButton=go

i can use chromedp.Navigate("http://testphp.vulnweb.com/search.php?test=query"), but how can i add the post data searchFor=dddd&goButton=go to the reques ？and custom headers Myheader: "example"

@xxxxnnxxxx I think you have the following options:

Just use a simple HTTP client to send the post request. Since you are asking here, I assume that it does not work for you. I write it down just in case you are not aware of it;

chromedp.Navigate acts mostly the same as inputting the URL in the address bar of the browser and then hitting enter. Namely,  it's supposed to send a GET request. It feels weird to change it to a POST request. I think what you should do is navigate to the page first, then input the search terms, and then submit the form. Check the example below:
package main

import (
	"context"
	"log"
	"time"

	"github.com/chromedp/cdproto/cdp"
	"github.com/chromedp/cdproto/fetch"
	"github.com/chromedp/chromedp"
)

func main() {
	opts := append(chromedp.DefaultExecAllocatorOptions[:],
		// uncomment the next line if your have something like Fiddler (to see the HTTP traffics)
		// chromedp.ProxyServer("http://localhost:8866"),
	)
	ctx, cancel := chromedp.NewExecAllocator(context.Background(), opts...)
	defer cancel()
	ctx, cancel = chromedp.NewContext(ctx,
		chromedp.WithDebugf(log.Printf),
	)
	defer cancel()

	chromedp.ListenTarget(ctx, func(ev interface{}) {
		switch ev := ev.(type) {
		case *fetch.EventRequestPaused:
			go func() {
				c := chromedp.FromContext(ctx)
				e := cdp.WithExecutor(ctx, c.Target)
				req := fetch.ContinueRequest(ev.RequestID)
				if ev.Request.Method == "POST" {
					// modify the headers here
					req.Headers = []*fetch.HeaderEntry{{"Myheader", "example"}}
				}
				if err := req.Do(e); err != nil {
					log.Printf("Failed to continue request: %v", err)
				}
			}()
		}
	})

	err := chromedp.Run(ctx,
		fetch.Enable(),
		chromedp.Navigate("http://testphp.vulnweb.com/search.php?test=query"),
		chromedp.WaitVisible("input[name=searchFor]", chromedp.ByQuery),
		chromedp.SendKeys("input[name=searchFor]", "dddd", chromedp.ByQuery),
		chromedp.Click("input[name=goButton]", chromedp.ByQuery),
		// wait for the POST request to finished. Do not use it in production code.
		chromedp.Sleep(time.Minute),
	)
	if err != nil {
		log.Fatal(err)
	}
}

I want to say more about option 1 in the previous comment. The point is running a browser instance is expensive. We should try our best to avoid that if we can do it with a simple HTTP client. If you want to modify the request from the very first, it seems that you don't need  anything special from the browser. In this case, maybe a simple HTTP client is a better choice.
package main

import (
	"context"
	"log"
	"net/http"
	"net/url"
	"strings"
)

func main() {
	formData := url.Values{
		"searchFor": {"dddd"},
		"goButton":  {"go"},
	}
	reader := strings.NewReader(formData.Encode())
	req, err := http.NewRequestWithContext(context.Background(), "POST", "http://testphp.vulnweb.com/search.php?test=query", reader)
	if err != nil {
		log.Fatal(err)
	}
	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
	req.Header.Set("Myheader", "example")
	resp, err := http.DefaultClient.Do(req)
	if err != nil {
		log.Fatal(err)
	}
	defer resp.Body.Close()
	log.Printf("response status: %v", resp.Status)
}

I found the issue; apologies here. chromedp predates the existence of the fetch API, and what's happening is a mutex is being locked during Navigate. This can be worked around, but appreciate the understanding here in that this requires a slight modification to chromedp's internal plumbing. I'll attempt to get a fix ASAP.

@kenshaw I have written the following demo to test this case.

deadlock demo
package main

import (
	"context"
	"log"

	"github.com/chromedp/cdproto/fetch"
	"github.com/chromedp/chromedp"
)

func main() {
	ctx, cancel := chromedp.NewContext(context.Background())
	defer cancel()

	chromedp.ListenTarget(ctx, func(ev interface{}) {
		switch ev := ev.(type) {
		case *fetch.EventRequestPaused:
			// uncomment the next line to avoid deadlock
			//go func(ctx context.Context, ev *fetch.EventRequestPaused) {
			if err := chromedp.Run(ctx, fetch.ContinueRequest(ev.RequestID)); err != nil {
				log.Println(err)
			}
			// uncomment the next line
			//}(ctx, ev)
		}
	})

	if err := chromedp.Run(ctx,
		fetch.Enable(),
		chromedp.Navigate("https://www.google.com/"),
	); err != nil {
		log.Fatal(err)
	}
}

The deadlock occurs because Target.run is holding Target.listenersMu and waiting for the event handler to finish (line 130 below), but the event handler calls Target.Execute (indrectly) to send a command, and Target.Execute waits for Target.listenersMu to add a listener so that it can got the response (line 179 below).
https://github.com/chromedp/chromedp/blob/d3ef975bdb93712fc152b4719300e19a84c6769f/target.go#L91-L131
https://github.com/chromedp/chromedp/blob/d3ef975bdb93712fc152b4719300e19a84c6769f/target.go#L162-L181
It seems that this is not specific to fetch. All the event handlers that call Target.Execute will result in a deadlock. It's easy to avoid it in the user code - just run the event handler in a new goroutine, and this has already been documented:
https://github.com/chromedp/chromedp/blob/d3ef975bdb93712fc152b4719300e19a84c6769f/chromedp.go#L675-L678
So it seems that we don't have to fix anything here. Is it okay to just update the document like this?
// Note that the function is called synchronously when handling events. The
// function should avoid blocking at all costs. For example, any Actions must be
// run via a separate goroutine (otherwise, it could result in a deadlock).

The document is updated. Closing.


Add PT-BR translation of Quickopen command

 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.:x: acwoss sign nowYou have signed the CLA already but the status is still pending? Let us recheck it.

I really liked the translation. My only suggestion is changing “teste unitário” to “teste de unidade”. I believe this translates better to Portuguese since we are testing a unit instead of executing a single test for everything. I know some literature has it wrongly translated (as many other classical wrong Brazilian translations in software) so I understand if kept the way it is. Congratulations for pull request.

@acwoss does the comment from @ppcmiranda need to still be addressed? Let me know as I'm ready to merge this when you give us the go-ahead.

@brettcannonYes, it's okay to merge.

Thanks everyone for this!


updated PhysiTech

adding electromagnetism to physitech and #3994

Close#3994

Checklist:

[x] I have mentioned the issue number in my Pull Request.
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have created a helpful and easy to understand README.md

@MohitGupta121 sir kindly review and merge this PR


standerize the types.ts file for instrumentations

following this PR, I noticed that some instrumentations has a types.ts file that contains both public and internal types mixed together.
I think this is not optimal, and want to suggest the following:

agree on a standart for how to structure the types in instrumentations (where to store internal / public types, how the files should be named, how to export them etc)
document it in the repo so new contributions / reviewers can use it as reference.
align all current instrumentations according to the standart.

If we get to an agreement, I can take the task to add documention markdown file to the repo and align all current instrumentations accordingly.
Public Types

Instrumentation config along with interfaces/types for hooks and filters (ignore matchers). example
Redecalring (copying) of types that we cannot import, because the package where they are declared includes implementation and is very large (aws-sdk).

A non backward compatible change in public types is breaking change and will be a problem once instrumentations are declared stable, so extra causion must be taken when adding types to this file. This is also why I think this change should be applied now while instrumentations are still experimental.
Internal Types
Meant to be consumed by instrumentation itself (instrumentation.ts, utils.ts etc) and should not be exported to instrumentation users. Changes can be made to these types which are guarenteed to not break users as they are not exported.

Types that extend some interface of the instrumented library itself. For example, adding new properties on Request for instrumentation purposes. exmaple
Declare types / interfaces used in the instrumentation logic itself. example

Options
Two Type Files (my personal preference)
one file for internal types and another file for public types.
Then, in index.ts we can export * from './types'; from the public types file, and in instrumentaiton.ts and other internal files we can import from the internal and public types files. This will help to prevent exporting internal types, and will introduce more structure into the codebase.
Suggestion for name: types.ts for public types. internal-types.ts for internal types. This is how winston instrumentation is built.
Single Types File
Put all types in a single types.ts file like we do now, but document that index.ts must export only public types and not *.
I am happy to get any comments and suggestions regarding this. We can possibly extend in the future to other aspects of instrumentation code structure and styling.

[x] This only affects the JavaScript OpenTelemetry library
[ ] This may affect other libraries, but I would like to get opinions here first

I prefer on two type files too so its more explicit (maybe add a comment on the public one that explains that you must not export internal types)


Allow configuring a time zone for a given room

Would be part of a generic room settings store for the bot.

#7


comma-style: add support for named exports and more

Clear and concise description of the problem
I've noticed that the comma-style rules don't support a lot of syntax yet.
For example,
let a, b, c;
// Named exports
export {
  a,
  b
  , c
};
// Comma operator
foo = (
  (a = 1),
  (b = 5)
  , (c = 6)
);

// TypeScript
type MyType<
  A,
  B
  , C
> = [
  string,
  string
  , string
];
enum MyEnum {
  "a",
  "b"
  , "c"
}
class MyClass implements
  A,
  B
, C {
}
function myEmptyFunction (
  a,
  b
  , c
);

// Import attributes
import json from "./foo.json" with {
  "a": "v",
  "b": "v"
  , "c": "v"
};
import(source,
  options);
import(source
  , options);

Suggested solution
Add support for unsupported syntax.

Currently the exceptions option for the comma-style rule seem to be based on the name of the AST node.
However, I find this design difficult to follow, as a single node may have multiple properties that list elements 🤔
What do you think? Do you have any ideas?
Alternative
No response
Additional context
No response
Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guide.
[X] Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Contributes

[X] If this feature request is accepted, I am willing to submit a PR to fix this issue

Not really related to this issue, but I looked into why this rule doesn't check the comma position for some syntax by default (e.g. ArrayPattern, ObjectPattern).
It seems like they just forgot to turn it on in a major version upgrade 👀
https://github.com/eslint/eslint/pull/7479#issuecomment-257714823

Currently the exceptions option for the comma-style rule seem to be based on the name of the AST node.
However, I find this design difficult to follow, as a single node may have multiple properties that list elements 🤔

If the exceptions option will be OFF (no exception) by default for all nodes in the future, I thought there was no need to worry too much about it.
Even if specifiers and attributes are made exceptions with exceptions.ImportDeclaration=true, it is the user's responsibility.


Changing peptide length filter

Hi,
Is it possible to change the filter for peptide length for predictions ?
Although the default (8 to 12 residues) can work well for some allotypes, others may include shorter or larger peptides (e.g. 7 to 13 residues).
Thanks for the great work on MhcVizPipe!
Best wishes,
David

Hi David,
Thanks for your interest in the tool. There are some limitations here, because we run everything through NetMHCpan and it has a minimum length requirement of 8 residues. This is something we might get around eventually by including other prediction tools...
However, there isn't anything preventing us from allowing the user to change the maximum length. Would that be useful enough to you on its own?
Best wishes,
Kevin

Hi Kevin,
Thanks for your reply. Yes, being able to include  peptides with up to 13 amino acids would be very helpful for us.
Best wishes,
David

Hi David,
Okay, sounds good. I will look into implementing this. Most likely it will end up in the settings popup window. If you think it would be more useful to have it as an option in the main GUI I can look into that (e.g. if you need to change it often), but it would be easier to put it in the settings.
Best,
Kevin

Hi Kevin,
I think that having the option in the settings popup would be the best.
Thanks a lot!
Best,
David

Hi David,
I just released a new version that should take care of this. You can update MhcVizPipe from within the GUI. You'll then need to update the settings, since the parameter was not present in the previous version. To do so, open the settings window and click the "LOAD DEFAULTS" button. This will add two parameters under the ANALYSIS section: class I max length and class II max length.
Sorry for the slow update on my part. COVID made its way into the household a couple weeks ago and everything has slowed down for a while...
Let me know if the new update works for you.
Best wishes,
Kevin

Hi Kevin,
Thanks a lot for adapting the settings to include larger peptides, that worked perfectly!
Sorry also for my very late reply. I tested that in a hurry and then forgot to update you. I hope that you recovered completely from COVID and everything goes fine for you.
Best wishes,
David


macOS support

TODO

[ ] local_events.txt, global_events.txt and Custom Resources attempt to load from wrong file paths
[x] keybinds should use cmd instead of ctrl
[ ] crash when opening midi chart due to "import vocal track?" dialog being presented on background thread
[x] CH export says files already exist when song is untitled
[ ] export ch package -> export -> choose folder -> override? dialog

upon hitting cancel, this error is spammed

[ ] export ch package dialog -> export button -> file browser

create a folder "aeiou"
"e" keyboard event is sent to game even though it is blocked and "extended sustains" is toggled

[ ] menu bar dropdowns open up on hover sometimes
[x] ~after a little bit there's extremely high CPU usage. makes my dual core macbook air scream.~Settings > Editor > Target FPS > 60 and disabling AA fixed this.

Details

Added macOS implementation of native window, file explorer, and message box

These implementations are written from scratch in Swift. The Xcode project is included.

Uses the FFmpeg OGG transcoding solution written for Linux on macOS as well.
Added BASS audio libraries for macOS
Added build command for macOS

Fixes #16

@bmwalters
Cmd keybind can be added into the KeyboardDevice under the ctrlKeyBeingPressed property and implemented into the ToModifierKey function, which should handle all input changes without having to rebind the default controls.

Note that Opus audio compatibility has been recently added and this will also need separate implementation here as well. Eventually will add the ability to export this audio format as well, using bass' opus audio encoder.


Wrong year in Visual Studio 2017/2019 Release Notes

Describe the bug
It is year 2020 now, NO LONGER 2019.
To Reproduce
Steps to reproduce the behavior:

Go to
https://docs.microsoft.com/en-us/visualstudio/releasenotes/vs2017-relnotes
https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes
https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes-preview

Look at the Release Notes release date, see the error

Expected behavior
year 2020.

This should be fixed - thank you for the feedback!


Prep for Release of v1.1.11

Tracking issue for the August 2024 release of wlbot.

wlbot v1.1.11 has been published to npm: https://www.npmjs.com/package/wlbot/v/1.1.11

wlbot v1.1.11 release has been created on GitHub: https://github.com/mike-weiner/wlbot/releases/tag/v1.1.11


Added sub-namespace separators

Overview
Namespaces did not have separators for sub-namespace when showing a variable type.
Nature of This Change

[ ] 🐛 Fixes a bug
[ ] 🚀 Implements a new feature
[x] 🛁 Rewrites existing documentation
[ ] 💅 Fixes coding style
[ ] 🔥 Removes unused files / code

Awesome, thanks so much!

No problem!


Bot won't recognise everyone's games

I've tried catching all my games via the presence event, but since the update, it only catches some users' games.
Upon evaling msg.author.game.name while playing any game, it presents me with the message TypeError: Cannot read property 'name' of null. The same applies to some other members of the server, but not all.
Side Note, How do I properly check if someone's playing a game via presence? I always do if (newMember.game !== null) {...}

The check is right.
Hmm...

I think we should just make user.game a string instead of an object so that it won't cause this error

Fixed in 9.0.2. user.game is an object as it can contain streaming data.


Going in circles: "Multiple commands produce..." error is solved with "pod install --repo-update" but then React-Codegen missing shadow nodes appears.

Description
Ok, so I'm going through the setup process for React Native Firebase and everything is working fine on Android. On iOS, after adding all the configurations I get a bunch of errors as follows:

So I got into the ios directory and solve it by running a pod install --repo-update. I go back to Xcode to build on the simulator and I get React Codegen error saying missing shadow nodes.

I return to my iOS folder and run RCT_NEW_ARCH_ENABLED=1 bundle exec pod install and that fixes the shadow nodes error but the Multiple commands produce... returns.
What is the proper way to handle these errors? Right now I'm just going in circles trying to fix both errors.
Version
0.70.1
Output of npx react-native info
System:
    OS: macOS 13.0
    CPU: (10) arm64 Apple M1 Max
    Memory: 423.63 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.9.0 - /opt/homebrew/bin/node
    Yarn: 1.22.18 - ~/.yarn/bin/yarn
    npm: 8.19.1 - /opt/homebrew/bin/npm
    Watchman: Not Found
  Managers:
    CocoaPods: 1.11.3 - /Users/vikrampal/.rbenv/shims/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 21.4, iOS 16.0, macOS 12.3, tvOS 16.0, watchOS 9.0
    Android SDK: Not Found
  IDEs:
    Android Studio: 2021.2 AI-212.5712.43.2112.8815526
    Xcode: 14.0/14A309 - /usr/bin/xcodebuild
  Languages:
    Java: 18.0.2 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: Not Found
    react-native: Not Found
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps to reproduce
The steps to reproduce are described above.
Snack, code example, screenshot, or link to a repository
In regards to me setting up Firebase, here are those files:
Podfile - I commented out Flipper as the docs recommended.
require_relative '../node_modules/react-native/scripts/react_native_pods'
require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'

$RNFirebaseAsStaticFramework = true

platform :ios, '12.4'
install! 'cocoapods', :deterministic_uuids => false

target 'MyApp' do
  config = use_native_modules!
  use_frameworks! :linkage => :static

  # Flags change depending on the env values.
  flags = get_default_flags()

  use_react_native!(
    :path => config[:reactNativePath],
    # Hermes is now enabled by default. Disable by setting this flag to false.
    # Upcoming versions of React Native may rely on get_default_flags(), but
    # we make it explicit here to aid in the React Native upgrade process.
    :hermes_enabled => true,
    :fabric_enabled => flags[:fabric_enabled],
    # Enables Flipper.
    #
    # Note that if you have use_frameworks! enabled, Flipper will not work and
    # you should disable the next line.
    # :flipper_configuration => FlipperConfiguration.enabled,
    # An absolute path to your application root.
    :app_path => "#{Pod::Config.instance.installation_root}/.."
  )

  target 'MyAppTests' do
    inherit! :complete
    # Pods for testing
  end

  post_install do |installer|
    react_native_post_install(
      installer,
      # Set `mac_catalyst_enabled` to `true` in order to apply patches
      # necessary for Mac Catalyst builds
      :mac_catalyst_enabled => false
    )
    __apply_Xcode_12_5_M1_post_install_workaround(installer)
  end
end

Ok, so I'm going through the setup process for React Native Firebase and everything is working fine on Android. On iOS, after adding all the configurations I get a bunch of errors as follows:

I would suggest you open this issue against React Native Firebase. @mikehardy would be more this happy to support you over there 👍 Thanks

Read the comments in this script, run it, and examine the podfile that results: https://github.com/mikehardy/rnfbdemo/blob/main/make-demo.sh
Start to finish + fully working react-native-firebase demo, debug + release, iOS + macCatalyst, with Hermes on react-native 0.70.1
Cheers

Worked like a charm! Thank you so much.


SplitAt (split a sequence in two at a specified index)

I'd like to propose a new operator that splits a sequence in two at a given index. The first sequence will contain all the items that appear before the index in the source sequence and the second will contain those items that appear at and after the index. A negative index would be allowed and would have the equivalent behavior of specifying an index of 0.
The follow table illustrates the splits that SplitAt will produce for a sample of indexes on a given source:

Source
Index
Split 1
Split 2

[1,2,3,4,5,6,7,8,9,10]
0
[]
[1,2,3,4,5,6,7,8,9,10]

[1,2,3,4,5,6,7,8,9,10]
2
[1,2]
[3,4,5,6,7,8,9,10]

[1,2,3,4,5,6,7,8,9,10]
5
[1,2,3,4,5]
[6,7,8,9,10]

[1,2,3,4,5,6,7,8,9,10]
10
[1,2,3,4,5,6,7,8,9,10]
[]

[1,2,3,4,5,6,7,8,9,10]
20
[1,2,3,4,5,6,7,8,9,10]
[]

[1,2,3,4,5,6,7,8,9,10]
-5
[]
[1,2,3,4,5,6,7,8,9,10]

Prototype
public static TResult SplitAt<T, TResult>(
    this IEnumerable<T> source, int index,
    Func<IEnumerable<T>, IEnumerable<T>, TResult> resultSelector) =>
    source.Index()
          .Partition(e => e.Key < index,
                     (xs, ys) => resultSelector(from x in xs select x.Value,
                                                from y in ys select y.Value));

Example
var xs = Enumerable.Range(1, 10);
var results =
    from i in new[] { 0, 2, 5, 10, 20, -5 }
    select xs.SplitAt(i, (h, t) => new
    {
        Index = i,
        Head  = "[" + string.Join(",", h) + "]",
        Tail  = "[" + string.Join(",", t) + "]",
    });

foreach (var result in results)
    Console.WriteLine(test);

Output
{ Index = 0, Head = [], Tail = [1,2,3,4,5,6,7,8,9,10] }
{ Index = 2, Head = [1,2], Tail = [3,4,5,6,7,8,9,10] }
{ Index = 5, Head = [1,2,3,4,5], Tail = [6,7,8,9,10] }
{ Index = 10, Head = [1,2,3,4,5,6,7,8,9,10], Tail = [] }
{ Index = 20, Head = [1,2,3,4,5,6,7,8,9,10], Tail = [] }
{ Index = -5, Head = [], Tail = [1,2,3,4,5,6,7,8,9,10] }

We circle back a bit to the discussion in #306 . What to do when the index is out of range? Here you propose to be permissive. In general I argue for strictness, because I prefer my program tell me when instructions are unclear instead of making guesses for me. Of course, I'm not /that/ strict on strictness (ha!), so I guess it depends on what the common use-cases would be. What use-cases do you see for this operator, that make permissiveness be a more sensible option?

We circle back a bit to the discussion in #306 . What to do when the index is out of range?

True. SplitAt permissive in the same way that Take and Skip are. Perhaps the problem here is using the word index rather than count and having at in the name. Not sure if calling it SplitHeads would have helped 😆 where the argument would have been the count of head elements desired in the first part followed by remaining tails elements in the second.

What use-cases do you see for this operator, that make permissiveness be a more sensible option?

If you're look at each part of the split as simple buckets then permissiveness is acceptable. If you run a Max on each bucket and one of them turned out empty (because index was out of range so to speak) then Max will throw. You get your strictness based on what you do with the buckets next. A Count, for example, would by happy with empty buckets.

Based on @leandromoh's feedback in PR #316, I am inclined to revise this so that instead of splitting a sequence in two and returning a pivoted result of the two parts, it's better to return a sequence of sequences where the sub-sequences start at the given offset into the source sequence and stop before the next. Below is an initial draft of what this could look like:
public static IEnumerable<IEnumerable<T>> SplitAt<T>(
    this IEnumerable<T> source, params int[] offsets)
{
    using (var oe = offsets.Concat(int.MaxValue).GetEnumerator())
    {
        oe.MoveNext();
        var offset = oe.Current;

        List<T> list = null;
        foreach (var e in source.Index())
        {
            retry:
            if (e.Key < offset)
            {
                if (list == null)
                    list = new List<T>();
                list.Add(e.Value);
            }
            else
            {
                yield return list ?? Enumerable.Empty<T>();
                offset = oe.MoveNext() ? oe.Current : -1;
                list = null;
                goto retry;
            }
        }

        if (list != null)
            yield return list;

        while (oe.MoveNext())
            yield return Enumerable.Empty<T>();
    }
}

This approach would be lazier and also support a source sequence that's infinite.
The initial example would now read as:
var xs = Enumerable.Range(1, 10);
var results =
    from i in new[] { 0, 2, 5, 10, 20, -5 }
    select xs.SplitAt(i).Fold((h, t) => new
    {
        Index = i,
        Head = "[" + string.Join(",", h) + "]",
        Tail = "[" + string.Join(",", t) + "]",
    });

foreach (var result in results)
    Console.WriteLine(result);

Note that Fold is used to effectively get convenient access to the two parts as before.


Update convert-disk-storage.md

Minor correction to Powershell scripts, so that it works with the latest Azure Powershell.

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


Add threading

Just spawn a task per box at the highest level and have them all accumulate their result somewhere should be enough, I think? Probably with a minimum box size to avoid spawning too many tasks, but the workload of each box should be big enough to avoid overhead.

I think it would be better to simply split the original domain into a grid and add threads there. We could add such a functionality here, but we can also simply have an example (or just say it?) to the docs.
Threading directly the algorithm can be a bit tricky due to race conditions in several places. Certainly doable, but not sure it is worth it.

Definitely makes sense, except load balancing is tricky (@threads :greedy is probably appropriate) so you'd need to have much more tasks than threads, which limits parallelism, so true parallelism would definitely be better (but also more of a hassle). Good to think about in relation with the idea to split the algorithm in two stages (determine the boxes, then run on them)

Yes, I think the two-stage method would be a good place to parallelise things, although parallelising the first pass of the algorithm, where bounds are computed and so on, will be tricky as well. In any case, for now I think the "split the domain and parallelise" seems like a reasonable approach/suggestion.


Feature Request: add add-network-policy to manifest

It is possible to set network policies using the command line:
cf add-network-policy SOURCE_APP --destination-app DESTINATION_APP --protocol (tcp | udp) --port RANGE

However it is not possible to set these values using the manifest. This makes it hard to perform deploys from automated systems.

cc @zrob

This is a really interesting request.
I don't know how we'll be able to accommodate it in the server side manifest world.  The network policy API is a totally separate entity from the CC.
/cc @rosenhouse @ameowlia @pvaramballypivot

@zrob what is this server-side-manifest-world that you speak of?  got a link?

The cli will no longer parse app manifests and translate them to API requests.  Instead they will send the yaml to CC and we'll apply it.
Epic: https://www.pivotaltracker.com/epic/show/3867153
API docs: http://v3-apidocs.cloudfoundry.org/version/3.55.0/index.html#app-manifest

@zrob this feature is pretty important, for example in case of the following situation:

When zero-downtime-updating App B, the following sequences are required:

cf rename app-b app-b-venerable
cf push app-b
cf add-network-policy --source app-a --destination app-b --port 8080
cf add-network-policy --source app-b --destination app-c --port 8080
cf delete -r -f app-b-venerable

Without a support from cf cli, autopilot plugin is unavailable and it is truoublesome for deploying cf apps in concourse

I really like the model of decorating the destination (server) app with the list of allowed source (client) apps.  +1 to this UX.

I see how that would be valuable.  We'd have to teach CC how to talk to policy server.  It has interesting implications for the create-manifest workflow as well.
I also wonder if there's some even simpler workflows if we went deep on CC/policy server integration
For example imagine something like

cf create-network networkName
cf add-app-to-network appName networkName
cf remove-app-from-network appName networkName

manifest something like this instead of using the add/remove commands
applications:
- name: app-b
  networks:
  - networkName
  - otherNetworkName

Would mean you don't have to keep a full list of apps all the time, although maybe the full list is desirable?

@zrob, it sounds like rolling deploys would fix @making's use case?

@zrob +1 for your idea.
My proposal will not work in case that App C is not deployed yet such as first deployment.
Adding the app to a logical group sounds pretty good

Will this be considered in the near future?

cc/ Networking PM @adobley

@kzbt this is not currently planned, but we have been collecting evidence to help us scope, plan, and prioritize this feature. It is in the pipeline, but I can't give an estimate as to when. If you have a case that isn't captured above, I'd love to hear about it.

@adobley Our usecase is the same/similar. We have a gateway app that controls all ingress and all internal traffic is using pcf internal routes. This makes add-network-policy a necessity for deployments. It is easy enough to automate it and we have done that in our ci pipelines, but problem arises when doing quick manual deploys to dev environments for testing etc.. because often this step is forgotten or devs don't realize it is required. Making it a declarative option in the manifest would be best as that would describe ideal state of the app in pcf once deployed.

Hi all, thanks for everyone's feedback. I'm moving this issue to the CC API's github repo since they are responsible for server side manifests for V3.


[Bug] ModuleNotFoundError: No module named 'timm' with Internal Vision model

Checklist

[x] 1. I have searched related issues but cannot get the expected help.
[x] 2. The bug has not been fixed in the latest version.

Describe the bug
docker run -d --restart=always --runtime nvidia --gpus '"device=6"' \
    -v $HOME/.cache/huggingface:/root/.cache/huggingface \
    --env "HUGGING_FACE_HUB_TOKEN=$HUGGING_FACE_HUB_TOKEN" \
    -p 23333:23333 \
    --ipc=host \
    openmmlab/lmdeploy:latest \
    lmdeploy serve api_server OpenGVLab/InternVL-Chat-V1-5

Fetching 33 files: 100%|??????????| 33/33 [00:55<00:00,  1.69s/it]
Traceback (most recent call last):
  File "/opt/py38/bin/lmdeploy", line 11, in <module>
    load_entry_point('lmdeploy', 'console_scripts', 'lmdeploy')()
  File "/opt/lmdeploy/lmdeploy/cli/entrypoint.py", line 37, in run
    args.run(args)
  File "/opt/lmdeploy/lmdeploy/cli/serve.py", line 303, in api_server
    run_api_server(args.model_path,
  File "/opt/lmdeploy/lmdeploy/serve/openai/api_server.py", line 1191, in serve
    VariableInterface.async_engine = pipeline_class(
  File "/opt/lmdeploy/lmdeploy/serve/vl_async_engine.py", line 20, in __init__
    self.vl_encoder = ImageEncoder(model_path, vision_config)
  File "/opt/lmdeploy/lmdeploy/vl/engine.py", line 69, in __init__
    self.model = load_vl_model(model_path)
  File "/opt/lmdeploy/lmdeploy/vl/model/builder.py", line 42, in load_vl_model
    return InternVLVisionModel(model_path, with_llm)
  File "/opt/lmdeploy/lmdeploy/vl/model/internvl.py", line 83, in __init__
    self.build_model()
  File "/opt/lmdeploy/lmdeploy/vl/model/internvl.py", line 93, in build_model
    model = AutoModel.from_config(config, trust_remote_code=True)
  File "/opt/py38/lib/python3.8/site-packages/transformers/models/auto/auto_factory.py", line 428, in from_config
    model_class = get_class_from_dynamic_module(class_ref, repo_id, **kwargs)
  File "/opt/py38/lib/python3.8/site-packages/transformers/dynamic_module_utils.py", line 510, in get_class_from_dynamic_module
    return get_class_in_module(class_name, final_module)
  File "/opt/py38/lib/python3.8/site-packages/transformers/dynamic_module_utils.py", line 208, in get_class_in_module
    module_spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 848, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/root/.cache/huggingface/modules/transformers_modules/fdc2259adb05898e81e9095757c8dde7b7c31470/modeling_internvl_chat.py", line 20, in <module>
    from .modeling_intern_vit import InternVisionModel
  File "/root/.cache/huggingface/modules/transformers_modules/fdc2259adb05898e81e9095757c8dde7b7c31470/modeling_intern_vit.py", line 12, in <module>
    from timm.models.layers import DropPath
ModuleNotFoundError: No module named 'timm'

Reproduction
above
Environment
docker

Digest: sha256:c2439c09f2432c72f99cfec814a91fbaa6f5d16d93ee1869a07109d37a9ab402
Status: Downloaded newer image for openmmlab/lmdeploy:latest

Latest image pulled just minute ago.
Error traceback
above

I guess this is normal problem, that I should build my own model-specific image?  Given the size of the image already, seems should be already in the image.
Each model may require specific dependencies not included in the Docker image. If you run into issues, you may need to install those yourself on a case-by-case basis. If in doubt, refer to the specific model’s project for documentation.

For example, for Llava:

FROM openmmlab/lmdeploy:latest

RUN apt-get update && apt-get install -y python3 python3-pip git

WORKDIR /app

RUN pip3 install --upgrade pip
RUN pip3 install timm
RUN pip3 install git+https://github.com/haotian-liu/LLaVA.git --no-deps

COPY . .

CMD ["lmdeploy", "serve", "api_server", "liuhaotian/llava-v1.6-34b"]

Yes. we recommend users build model-specific images by themselves
Please refer to the following guide for an explanation
https://lmdeploy.readthedocs.io/en/latest/serving/api_server_vl.html#option-2-deploying-with-docker


Refactor by applying the "Mutex hat" idiom

The PR improves readability by applying the "Mutex hat" idiom. See also this slide.
Additionally, this PR renames mutexes names by using the Mu suffix.

I'm wondering if it would make sense to group protected fields together with their mutex into their own struct. That would avoid repeating some of the redundant naming, and makes it even more obvious which fields are protected by which mutex.
Something like this (untested):
type worthCheckingIPTables struct {
	sync.RWMutex
	value bool
}

type latestIPTables struct {
	sync.RWMutex
	entries []iptables.Entry
}

type agent struct {
	worthCheckingIPTables
	latestIPTables
}

func foo(a *agent) {
	a.worthCheckingIPTables.Lock()
	a.worthCheckingIPTables.value = true
	a.worthCheckingIPTables.Unlock()
}

This is only meant as a RFC, I'm not sure if this works better or not.

I'm not sure if we will gain any benefit by adding a struct that contains a mutex and protected fields. We have only 7 mutexes in the repo, which is a small amount.
The Go codebase doesn't introduce a separate struct for mutexes, so we shouldn't either.

The Go codebase doesn't introduce a separate struct for mutexes, so we shouldn't either.

Just briefly browsing those search results it looks like almost half of them show a struct with a mutex at top (most often called just mu) that protects the whole struct.
An exact example of the pattern I'm discussing here can be seen at https://github.com/golang/go/blob/81c92352a7c6aadc434e7d0921d046a599ba2807/src/os/signal/signal.go#L13.

On the other hand, Uber Go Style Guide is against embedding mutexes:

Exception: Mutexes should not be embedded, even on unexported types.

On the other hand, Uber Go Style Guide is against embedding mutexes:

Exception: Mutexes should not be embedded, even on unexported types.

The Goland may warn about embedded mutexes when the team implements https://youtrack.jetbrains.com/issue/GO-14143.

I honestly don't understand the argument how foo.mu.Lock() is good but foo.Lock() is bad. In both cases you expose the fact that you need to lock the struct before you should access the fields. And it is not an internal implementation detail; it is part of the contract on how you can use the struct.
I do get the argument that you should use one style consistently, but it seems arbitrary which one you pick.
Anyways, I'm fine with naming the mutext mu and not use embedding. I just don't want someThingVeryLongAndHardToRead and another someThingVeryLongAndHardToReadMu field that I have to lock.

I honestly don't understand the argument how foo.Mu.Lock() is good but foo.Lock() is bad.

Ok, I do get the argument if all the protected fields are non-exported fields. In that case you should not expose the locking mechanism either. But as soon as any protected fields are exported, you must export the locking mechanism too.
And none of this should apply to non-exported types.

Let's postpone this.


Add root CAs

I'm not sure if they are necessary in both images but I don't see any harm in having them.
Fixes #1176.

👍 for adding root certificates into API server.

I'm not sure if they are necessary in both images but I don't see any harm in having them.

I don't think we need them for controller-manager, since there is no API.
I don't see any benefit of adding extra stuff if we don't need it.

Will LGTM once you leave this change only for API server.

Actually I think we need it for controller-manager since it makes calls to the brokers.
Not sure if we need to tweak the broker config for that though, will check later.

So the code in the broker client registering certificates is there: https://github.com/pmorie/go-open-service-broker-client/blob/master/v2/client.go#L50
	if len(config.CAData) != 0 {
		if transport.TLSClientConfig.RootCAs == nil {
			transport.TLSClientConfig.RootCAs = x509.NewCertPool()
		}
		transport.TLSClientConfig.RootCAs.AppendCertsFromPEM(config.CAData)
	}
	if transport.TLSClientConfig.InsecureSkipVerify && transport.TLSClientConfig.RootCAs != nil {
		return nil, errors.New("Cannot specify root CAs and to skip TLS verification")
	}

not sure whether transport.TLSClientConfig.RootCAs will be correctly initialized if ClusterServiceBrokerSpec.CABundle == nil

@ash2k do you know if we need to explicitly configure root CAs for net/http/Transport for using available root CAs that you installed using apt-get install ca-certificates (see
the comment above)?

If cabundoe is nil golang uses the root cas, afaik
On Tue, Oct 17, 2017 at 10:03 PM Nail Islamov<EMAIL_ADDRESS>wrote:

So the code in the broker client registering certificates is there:
https://github.com/pmorie/go-open-service-broker-client/blob/master/v2/client.go#L50
if len(config.CAData) != 0 {
if transport.TLSClientConfig.RootCAs == nil {
transport.TLSClientConfig.RootCAs = x509.NewCertPool()
}
transport.TLSClientConfig.RootCAs.AppendCertsFromPEM(config.CAData)
}
if transport.TLSClientConfig.InsecureSkipVerify && transport.TLSClientConfig.RootCAs != nil {
return nil, errors.New("Cannot specify root CAs and to skip TLS verification")
}
not sure whether transport.TLSClientConfig.RootCAs will be correctly
initialized if ClusterServiceBrokerSpec.CABundle == nil
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes-incubator/service-catalog/pull/1419#issuecomment-337438688,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWXmLTVsFvz9TBpIj8vj_X3VeT1Nbybks5stVx-gaJpZM4P9AmY
.

I mean, uses the built in cas from the system trust store
On Tue, Oct 17, 2017 at 10:15 PM Paul Morie<EMAIL_ADDRESS>wrote:

If cabundoe is nil golang uses the root cas, afaik
On Tue, Oct 17, 2017 at 10:03 PM Nail Islamov<EMAIL_ADDRESS>wrote:

So the code in the broker client registering certificates is there:
https://github.com/pmorie/go-open-service-broker-client/blob/master/v2/client.go#L50
if len(config.CAData) != 0 {
if transport.TLSClientConfig.RootCAs == nil {
transport.TLSClientConfig.RootCAs = x509.NewCertPool()
}
transport.TLSClientConfig.RootCAs.AppendCertsFromPEM(config.CAData)
}
if transport.TLSClientConfig.InsecureSkipVerify && transport.TLSClientConfig.RootCAs != nil {
return nil, errors.New("Cannot specify root CAs and to skip TLS verification")
}
not sure whether transport.TLSClientConfig.RootCAs will be correctly
initialized if ClusterServiceBrokerSpec.CABundle == nil
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes-incubator/service-catalog/pull/1419#issuecomment-337438688,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWXmLTVsFvz9TBpIj8vj_X3VeT1Nbybks5stVx-gaJpZM4P9AmY
.

@pmorie yeah, sounds like it. Than the PR is good to merge, I think.

LGTM.


update release instructions

git push does not push tags. Need to run git push to push changelog commits and git push --tags to push tags.

Does git push --tags only push the tags, and not any commits?

@ncphillips I think so

yes