encoding/protojson: marshaled element in array/map and []byte(nil) to zero value instead with null

What version of protobuf and what language are you using?
Version: v1.28.1
What did you do?
syntax = "proto3";

package test.v1;

import "google/protobuf/wrappers.proto";

message Message {
  string id = 1;
}

message CaseNull {
  bytes b1 = 1;
  bytes b2 = 2;
  repeated bytes rpt_b = 3;
  map<string,bytes> map_b = 4;
  repeated google.protobuf.Int32Value rpt_wkt_i32 = 5;
  map<string,google.protobuf.Int32Value> map_wkt_i32 = 6;
  repeated Message rpt_msg = 7;
  map<string,Message> map_msg = 8;
}

func TestCaseNull(t *testing.T) {
	m := &testv1.CaseNull{
		B1:   nil, // marshaled to "" instead with null
		B2:   []byte(`abc`),
		RptB: [][]byte{[]byte(`ABC`), nil, []byte(``), []byte(`EFG`)},
		MapB: map[string][]byte{"keyA": nil, "keyB": []byte(`HIJ`)},
		RptWktI32: []*wrapperspb.Int32Value{
			wrapperspb.Int32(-1),
			wrapperspb.Int32(0),
			nil, // marshaled to 0 instead with null
			wrapperspb.Int32(1),
		},
		MapWktI32: map[string]*wrapperspb.Int32Value{
			"a": nil, // marshaled to 0 instead with null
			"b": wrapperspb.Int32(0),
		},
		RptMsg: []*testv1.Message{
			&testv1.Message{Id: "id1"},
			nil, // marshaled to {"id":""} instead with null
			&testv1.Message{Id: "id3"},
		},
		MapMsg: map[string]*testv1.Message{
			"msgA": &testv1.Message{Id: "ida"},
			"msgB": nil, // marshaled to {"id":""} instead with null
			"msgC": &testv1.Message{Id: "idc"},
		},
	}

	jsn, _ := protojson.Marshal(m)
	log.Println(string(jsn))
}

What did you expect to see?
{"b1":"", "b2":"YWJj", "rptB":["QUJD", null, "", "RUZH"], "mapB":{"keyA":null, "keyB":"SElK"}, "rptWktI32":[-1, 0, null, 1], "mapWktI32":{"a":null, "b":0}, "rptMsg":[{"id":"id1"}, null, {"id":"id3"}], "mapMsg":{"msgA":{"id":"ida"}, "msgB":null, "msgC":{"id":"idc"}}}

What did you see instead?
{"b1":"", "b2":"YWJj", "rptB":["QUJD", "", "", "RUZH"], "mapB":{"keyA":"", "keyB":"SElK"}, "rptWktI32":[-1, 0, 0, 1], "mapWktI32":{"a":0, "b":0}, "rptMsg":[{"id":"id1"}, {"id":""}, {"id":"id3"}], "mapMsg":{"msgA":{"id":"ida"}, "msgB":{"id":""}, "msgC":{"id":"idc"}}}

Anything else we should know about your project / environment?
	jsn, _ = json.Marshal(struct {
		B    [][]byte
		Msgs []*testv1.Message
	}{
		B: [][]byte{
			[]byte(nil),
		},
		Msgs: []*testv1.Message{
			nil,
		},
	})
	fmt.Print(string(jsn))
	// Output:
	// {"B":[null],"Msgs":[null]}

protojson can't even unmarshal such json now
	m.Reset()
	err := protojson.Unmarshal([]byte(`{"rptB":["QUJD",null,"","RUZH"]}`), m)
	fmt.Print(err)
	// Output:
	// proto: (line 1:17): invalid value for bytes type: null

This is working as intended. See https://github.com/protocolbuffers/protobuf-go/releases/tag/v1.20.0#v1.20-nil-values for details.

This is working as intended. See https://github.com/protocolbuffers/protobuf-go/releases/tag/v1.20.0#v1.20-nil-values for details.

Is fuzzy unmarshal acceptable?
For example, 64bit integers will be marshaled to string type, but json.Number can still be properly unmarshaled now.
Even if the nil value existing in the map/list will be marshaled to a zero value, is it acceptable that json.NullValue can be unmarshaled to a default value?
I understand that the current way is for consistency, marshalA=>unmarshal=>marshalB(expect marshalA==marshalB instead with A==B).
But if it's for consistency.

Since we can marshal []type{"a",null,"c"} to ["a","","c"] , should we also accept unmarshal ["a",null,"c"] to []type{"a","","c"}
or if rejecting unmarshal ["a",null,"c"] to []type{"a","","c"} , shouldn't that also reject marshal []type{"a",null,"c"} to ["a","","c"]

Another case:
	&testv1.CaseValue{
		Vs: []*structpb.Value{
			structpb.NewNullValue(),
			nil,
			structpb.NewBoolValue(false),
		},
	}

This results in an error google.protobuf.Value: none of the oneof fields is set
Shouldn't the zero value of google.protobuf.Value bestructpb.NewNullValue()?

Another case:
	&testv1.CaseValue{
		Vs: []*structpb.Value{
			structpb.NewNullValue(),
			nil,
		},
	}

This results in an error google.protobuf.Value: none of the oneof fields is set Shouldn't the zero value of google.protobuf.Value bestructpb.NewNullValue()?

https://developers.google.com/protocol-buffers/docs/reference/google.protobuf#google.protobuf.Value
A producer of value is expected to set one of that variants, absence of any variant indicates an error.

I just encountered this problem when empty slice was parsed as nil for some reason. Which pissed me off when everywhere says to move from golang protobuf to this google implementation. I had to switch to simple json marshaler to get the expected behavior.


Android - Split - Input field remains 0 when resetting split amount when the field is focused

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number: 1.4.72-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team
Action Performed:

Launch New Expensify app
Go to group chat
Go to + > Split expense
Enter amount > Next
On confirmation page, tap on the split amount field and clear the field
Dismiss the keyboard, and the input field will remain focused
Tap Reset

Expected Result:
The input field which is focused will show the original split amount
Actual Result:
The input field still remains 0 when resetting split amount when the field is focused
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[x] Android: Native
[ ] Android: mWeb Chrome
[x] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/b828f1ba-e9b5-492e-9cbd-92dbb9d423ed
View all open jobs on GitHub

@joekaufmanexpensify  FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

We think that this bug might be related to #vip-split

Proposal
Please re-state the problem that we are trying to solve in this issue.
The split amount reset doesn't work if the field is focused. While the issue title mentions only 0, it could be any value.
What is the root cause of that problem?
In the MoneyRequestAmountInput, we have a check in the following line
https://github.com/Expensify/App/blob/974f6b58e0acdebd1f97e8c712df0c7a267b9470/src/components/MoneyRequestAmountInput.tsx#L181-L186
which returns early when the current text input is focused textInput.current?.isFocused(). This causes the early return and we don't end up calling setCurrentAmount.
What changes do you think we should make in order to solve the problem?

We need to retain textInput.current?.isFocused() because if we type and the amount keeps changing, then setCurrentAmount will cause flickers and frequent formatting of the amount while the user types. Hence, instead of returning early when the text input is focused, we should add a debounce to the method to execute it with a delay. This will ensure that the setCurrentAmount will be called but only when the user stops typing.
What alternative solutions did you explore? (Optional)
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

Proposal
Please re-state the problem that we are trying to solve in this issue.

The input field still remains 0 when resetting split amount when the field is focused

What is the root cause of that problem?

We have inconsistent behavior on web and native when focusing on any input and then clicking on "Reset".

On web: Clicking on "Reset" button will blur the input, which leads to the below condition will return false (textInput.current?.isFocused(): false):
https://github.com/Expensify/App/blob/5730ba4dced381adc473e502f675e7cf4e974a19/src/components/MoneyRequestAmountInput.tsx#L182
so the amount is reset.

On native: Clicking on "Reset" button will not blur the input, which leads to the below condition will return true (textInput.current?.isFocused(): true):
https://github.com/Expensify/App/blob/5730ba4dced381adc473e502f675e7cf4e974a19/src/components/MoneyRequestAmountInput.tsx#L182
so the amount is not reset.

What changes do you think we should make in order to solve the problem?

If the expected behavior is The input field still remains 0 when resetting split amount when the field is focused, we should update this:
https://github.com/Expensify/App/blob/5730ba4dced381adc473e502f675e7cf4e974a19/src/components/OptionsList/BaseOptionsList.tsx#L223
to:

                        <PressableWithFeedback onMouseDown={(e) => {
                                e.preventDefault();
                            }} ... />

We can apply the above change conditionally.

What alternative solutions did you explore? (Optional)

NA

I updated proposal

Will triage today

I don't think this is really a bug. Every other field in the app I tested works like this if you manually dismiss the keyboard. If you tap on the field again, we open the keyboard again. Closing as this doesn't seem to be a bug IMO.

@joekaufmanexpensify There is an inconsistency between the android-web and android-native:

In android web, click on 'Reset' will reset the amount.

In android native, click on 'Reset' will not reset the amount

Do you think it is a bug? Pls help check when you have a chance.

Could you clarify what you mean by clicking on "reset"?

I mean the "Reset" button in step 7:

@joekaufmanexpensify Do you have any feedback about this?

When you click out of the field that is 0, is it resetting then? It was for me when I had tested it previously. So it is resetting, the field was just not live updating.


Install issues

Required Info

Camera Model
F200

Firmware Version

Operating System & Version
Win 10 x64

librealsense version
Not 2 as it doesnt support my camera

pyrealsense version
Whatever's on pip?

Is this still maintained?
Anyway I tried to install using,
pip install pyrealsense
But got the following error
ERROR: Command errored out with exit status 1: 'D:\ProgramData\Anaconda3\python.exe' -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'C:\\Users\\NICBWT~1\\AppData\\Local\\Temp\\pip-install-25uygt1_\\pyrealsense\\setup.py'"'"'; __file__='"'"'C:\\Users\\NICBWT~1\\AppData\\Local\\Temp\\pip-install-25uygt1_\\pyrealsense\\setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record 'C:\Users\NICBWT~1\AppData\Local\Temp\pip-record-1q72eywi\install-record.txt' --single-version-externally-managed --compile --install-headers 'D:\ProgramData\Anaconda3\Include\pyrealsense' Check the logs for full command output.
Any idea whats going on here?

My apologies I didnt see the earlier response!


⚠ ClamAV has degraded performance

In 515507e, ClamAV (https://spamassassin.apache.org/) experienced degraded performance:

HTTP code: 200
Response time: 233 ms

Resolved: ClamAV performance has improved in 6ff3a84.


Can set primary output/display?

Is there an option to set the primary display, like I could with xrandr?
xrandr --output HDMI1 --mode 1680x1050 --pos 0x0 --rotate normal --output HDMI2 --primary --mode 1680x1050 --pos 1680x0 --rotate normal --output HDMI3 --mode 1680x1050 --pos 3360x0 --rotate normal

I see there's a 'primary' description here: https://github.com/SirCmpwn/sway/blob/f64b7e7/sway/ipc.c#L444
but there isn't any documentation saying I can, for example, write:
output HDMI1 resolution 1680x1050 position 0,0
output HDMI2 primary resolution 1680x1050 position 1680,0
output HDMI3 resolution 1680x1050 position 3360,0

in sway config?

There's no such thing as a primary display on Wayland.

Ah, derp. Well then, I wonder how swaybar will decide what output to be on... Probably an explicit setting, I'll bet.

Correct.

Excellent. This will serve for posterity, anyone else wanting to know some more differences between wayland and X output handling.
Thank you for your work!


Suggestion: Allow clean command to be used postfix, to be consistent with compile

Currently if you want to compile, test, or run a custom command on a project, you do this (examples from Indigo):
./mill demos.campfire.compile
./mill demos.campfire.test
./mill demos.campfire.runGame

..but if you want to clean it, you have to do this:
./mill clean demos.campfire

It would be really nice, if you could additionally use the same syntax as you use for everything else:
./mill demos.campfire.clean

I don't think this is a good idea, as it breaks with the perceived regularity of the hierarchical build structure and the associated CLI arguments. Just from looking at ./mill demos.campfire.clean I would say clean is a target or command and demo.campfire the module, but that wouldn't hold any longer. Also, once we introduce something like that and use it, there is a high risk users are confused about the difference is between them.

I suspect that's because you perceive, or know, a [technical] difference between the two.
I don't know, as a user, that there's any difference other than that it's irregular compared to all the other instructions I can issue to a module, and I have to remember that difference, and cleaning the module is a faff. I can't just hit up on my terminal and change compile to clean, I have to remove .compile, move to the start of the line, and insert clean and then hit enter. Doesn't sound like much but.. 😬
Anyway, your call, of course. I guess I can always set up a custom command or something.

The clean command accepts a target pattern, so you have very fine control what to clean. E.g. you said you need to remove the .compile from your command, but if you just want to clean the compilation result, you can and should keep it: ./mill clean demos.campfire.compile will clean only result of the demos.campfire.compile target. You can clean any target, or a pattern. E.g. to clean all compilation results but nothing else (e.g. preserve dependency resolutions), you can run ./mill clean __.compile.


Skeleton Initialization files to ES Module format

Hi,
Love the branch, only issue I had when I forked was the babel.config.js and jest.config.js files. The exports should be as an ES module, rather than a CommonJS module.

Fixed! https://github.com/ramipellumbi/NodeJS-Server-Template/commit/7abdcf83271a2fe27343f4a18dc831d22f3d9ecf


Make WinP Debuggable

[x] - Start building JAR bundle with Debug versions of DLLs (symbols will be published on AppVeyor)
[x] - Explicitly specify mvn package versions in the build flow
[x] - Rework build.cmd to take more parameters
[x] - In Appveyor builds use the locally defined version

@reviewbybees

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@reviewbybees done
Batch was definitely a wrong choice, I have never planned to make it complex. Makes sense to consider powershell for the next big refactoring anyway.

Merging to get the CI changes applied


{MDS,Joint}Writer should be more tolerant to transient failures in remote storage

Environment

python 10
rev: 764a73014ca882aba372aad9c454dca73f07199d
OS: apache/beam_python3.10_sdk:2.49.0
n2d-standard, etc. (GCP)

It is a bit of a challenge to reproduce this behavior, so I'll do my best to explain what I believe to be happening.
We've been following the documentation here to create a massive dataset using the MDS format! I noticed that the writer (Joint and Base class) asynchronously uploads index and shard data here and here. Additionally, because we are on GCP, we use GCS. I've noticed that the GCSUploader uses google storage client to perform the upload, which is great!
Our problem can be described as following: there is a rare situation where it's possible to write an index file to remote storage, where a shard contained in the index fails to upload. Thus the index is incorrect or corrupt.
I believe it precipitates as follows

In the call to finish the writer, both the in flight data and index are flushed and enqueued to be written to remote storage
There is no guarantee on the order in which these uploads happen
The above gcs uploader is not configured to retry on failure
The index upload succeeds
The data shard upload fails
The program won't terminate because the future isn't explicitly handled in the main thread, an error is simply logged

If folks agree with this characterization, then there are some solutions:

enqueue the index upload only after any inflight upload requests are completed. When this happens, the event will be set, and the index file will, correctly, not be uploaded.
configure the gcs uploader to retry on failures
explicitly handle futures and hard fail the main thread

Hey @thempatel, thanks for raising this issue. Will discuss with the team and get back to you!

Hey @thempatel, the team's aware of the issue and we're currently working on implementing better retry logic for both downloaders and uploaders. This will address the issue you're seeing. Thanks for bringing this up with us!

Hi @thempatel , Can you please review this PR https://github.com/mosaicml/streaming/pull/448 to see if the issues that you have mentioned above solves that or not? Thanks!

@snarayan21 @karan6181 thanks team! the attached change looks 👌🏽 !

@karan6181 Hi, thanks for fixing this issue. Since there is no new released version for this new feature, how can I use it?

Hi @viyjy , we are working on a new patch release. In the mean time, can you use main branch ?

Hi @viyjy , we are working on a new patch release. In the mean time, can you use main branch ?

Thanks. I have already launched multiple jobs using previous version, and they are running now. I can see files are saving to my s3 bucket. What if my current jobs failed at some moment? Should I delete all the generated files on s3 and then use main branch to generate data from scratch? Since my dataset is very large, I am trying to avoid rewriting files that are already saved to s3. Thanks.

Hi @viyjy , we are working on a new patch release. In the mean time, can you use main branch ?

Update: all of my jobs failed due to this error https://github.com/mosaicml/streaming/issues/453


Update processOrphanBlockPoS To Use New Validator Set Fetch Helper

[!WARNING]
This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

Current dependencies on/for this PR:

feature/pos-syncing-and-steady-state

PR #918

PR #919

PR #920   👈

This stack of pull requests is managed by Graphite.


rohmu/google: support connectivity through proxies

Support operations over proxies. The support is implemented only
for the Google storage at this time. Proxy information is delivered
in the config, e.g.
{
    "proxy_info": {
        "type": "socks5",
        "host": "proxy.example.com",
        "port": 1080,
        "user": "username",
        "pass": "password"
    }
}

Currently testing this locally with actual proxies. There are some changes I need to do before being ready.


Can not make install grpc in Centos 7

Hi,
I could not get over with this error when make install grpc in Centos 7, please help. I also make install protobuf before make and make install grpc but it not success. (make verify-install could not help also).
We couldn't find protoc 3.0.0+ installed on your system. While this
won't prevent grpc from working, you won't be able to compile
and run any meaningful code with it.
Please download and install protobuf 3.0.0+ from:
https://github.com/google/protobuf/releases
Once you've done so, or if you think this message is in error,
you can re-run this check by doing:
Here is protobuf in my system
[root@gonzo rasdaman]# ls -l /usr/local/lib | grep 'proto*'
-rw-r--r--. 1 root root 51965098 Nov  5 13:39 libprotobuf.a
-rwxr-xr-x. 1 root root      901 Nov  5 13:39 libprotobuf.la
-rw-r--r--. 1 root root  4306170 Nov  5 13:39 libprotobuf-lite.a
-rwxr-xr-x. 1 root root      916 Nov  5 13:39 libprotobuf-lite.la
lrwxrwxrwx. 1 root root       26 Nov  5 09:58 libprotobuf-lite.so -> libprotobuf-lite.so.10.0.0
lrwxrwxrwx. 1 root root       26 Nov  5 09:58 libprotobuf-lite.so.10 -> libprotobuf-lite.so.10.0.0
-rwxr-xr-x. 1 root root  1908482 Nov  5 09:58 libprotobuf-lite.so.10.0.0
lrwxrwxrwx. 1 root root       21 Nov  5 09:58 libprotobuf.so -> libprotobuf.so.10.0.0
lrwxrwxrwx. 1 root root       21 Nov  5 09:58 libprotobuf.so.10 -> libprotobuf.so.10.0.0
-rwxr-xr-x. 1 root root 20500151 Nov  5 09:58 libprotobuf.so.10.0.0
-rw-r--r--. 1 root root 86971888 Nov  5 13:39 libprotoc.a
-rwxr-xr-x. 1 root root      925 Nov  5 13:39 libprotoc.la
lrwxrwxrwx. 1 root root       19 Nov  5 09:58 libprotoc.so -> libprotoc.so.10.0.0
lrwxrwxrwx. 1 root root       19 Nov  5 09:58 libprotoc.so.10 -> libprotoc.so.10.0.0
-rwxr-xr-x. 1 root root 29639716 Nov  5 09:58 libprotoc.so.10.0.0

The default installation for Centos 7 of protobuf appears to be version 2.5. Please confirm by running protoc --version.
Furthermore, the libraries should still have been successfully built, as we do include a local copy of the required protobuf version. You'll simply be unable to invoke the plug-ins (conveniently, anyway - the protoc we build locally should still be usable).

protoc --version
libprotoc 3.0.0
I see it is not good when both products from Google but they are not compatible (!). The problem is when I install some program that requires grpc, it said that grpc is not installed then I think protobuf has been installed correctly but the problem is in grpc.

Which program are you installing that requires GRPC and did you ever actually install GRPC in spite of the warning provided by the Makefile?

A seperate install of the protobuf master branch and  a "export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib" works ...
I'm using GRPC in Centos 7 with Golang and NodeJS!

It seems grpc has some problems detecting the installation of protobuf. @js1990's comment might have pointed out the right cause. grpc's error message could be more clear about it. Something like "Found protoc in PATH but couldn't find libprotobuf in the default library path." may help.

This has been solved by the user.


Update blast error message for failed submission

Description
Change the blast error message for all jobs failing.
XD: https://xd.adobe.com/view/f585eb32-fbdd-4233-9451-bbb965678c1c-a0d6/screen/0c0b6eed-35b4-4b91-9ccb-de8899af197b?fullscreen
NOTE: The changes proposed in the ticket were already available except for blast submission error message. Hence, the one liner for this PR.
Related JIRA Issue(s)
ENSWBSITES-1836
Deployment URL(s)
http://update-blast-error-messages.review.ensembl.org
Views affected
BLAST

From the discussion with Andrea today, we found out the following changes need to be made:

Submission failed (track API): Submission failed
Submission failed after all jobs failed (jDispatcher): All jobs have failed individually
Some job(s) failed: Results no longer available along with the job failed message for the sequence

Here's something we missed in Jyo's PR

If a single job in a list of jobs grouped by sequence fails, you can see that the name of the species is greyed out (i.e. inactive); but there is still a show/hide chevron next to the species name (which does nothing), and the cursor changes to a pointer when mousing over the grey species name, misleadingly suggesting that you can click on it.
Up to you whether you would like to be fixed in this PR, or in a different one.


Introduce not given and apply it to dataset's target

Description
Introduce not given and apply it to dataset's target
Related Issue

Type of Change

[ ] 📚 Examples / docs / tutorials / dependencies update
[ ] 🔧 Bug fix (non-breaking change which fixes an issue)
[x] 🥂 Improvement (non-breaking change which improves an existing feature)
[ ] 🚀 New feature (non-breaking change which adds functionality)
[ ] 💥 Breaking change (fix or feature that would cause existing functionality to change)
[ ] 🔐 Security fix

Checklist

[ ] I've read the CODE_OF_CONDUCT.md document.
[ ] I've read the CONTRIBUTING.md guide.
[ ] I've updated the code style using make codestyle.
[ ] I've written tests for all new methods and classes that I created.
[ ] I've written the docstring in Google format for all the methods and classes that I used.

Looks good to me @kevinmessiaen.
Could we also undo the changes done in https://github.com/Giskard-AI/giskard/pull/1531/files (the ones related to suppress the warning of target for LLM) and add the target=None in the BaseModel for when model_type == "text_generation" to suppress the warning?

Not sure to understand the logic of the validation though, why to we validate the dataset target when validating model?
For me this validation is completely unrelated to the model validation and should be done when validating dataset only.
For me when uploading a model we should not confuse the user with warning that the dataset is missing a target, since it's unrelated.

that's exactly what I meant by my comment. We should remove it.
We had to do it to suppress the warning only for LLMs. We needed to do it inside validate_model just to know the model_type back then. But now people can put explicitly target=None to supress this warning

that's exactly what I meant by my comment. We should remove it. We had to do it to suppress the warning only for LLMs. We needed to do it inside validate_model just to know the model_type back then. But now people can put explicitly target=None to supress this warning

Thanks for clarification, so it's what I did then, you can review the PR when you have time


Perfect forwarding should be followed by std::forward

I have had crashes when I tried to use Single in multithreaded environment. With debugging and testing, I saw that if we don't do perfect forwarding properly we lose objects (the function pointer objects.
So I have replaced this code in Single.h
  void subscribe(Success&& next) {
    subscribe(SingleObservers::create<T>(next));
  }

with this
  void subscribe(Success&& next) {
    subscribe(SingleObservers::create<T>(std::forward<Success>(next)));
  }

and it fixed the crash!
So I have applied the same to all locations where std::forward should have been used.

While I agree this PR is an improvement, I wonder if an even better course of action is to take arguments by value and move them instead (i.e. Next next;  flowable->subscribe(std::move(next));).
We can totally get in trouble by accidentally taking something by reference we don't mean to with perfect forwarding, these methods feel like they should be taking ownership of whatever callable we're passing in.

I will revert the moving of rvalue reference lines ;)


Implement Data Retrieval Logic

Link to task on trello here

Works perfectly, code looks great
Screenshot of the table with metrics


Fix link typo for language handling in WN for 2004

Fix link typo for language handling in WN for 2004

#sign-off (Done with changes, ready for review)

#sign-off (Done with changes, ready for review)


39.00.26 - Summoned carths do not have their names updated

Walk around west of Maschunn in Chapter 2 until encountering 2 carths and an erabi. Kill one of the carths, and wait for the other carth to call for help. Note the name of the new combatant.

This issue also occurs in the original text.


Move plugins in other repositories

For correct use of scaleApp plugins in a Node.js/CommonJS world, is-it possible to move each plugin to a single repository ?

is-it possible to move each plugin to a single repository ?

of course it is possible ;-) ... but it might be a bit inflated. If there would be hundreds of people using scaleApp it makes sense but for such a small project I can't see the additional value.
So what would be the specific benefits?

Managing modules dependancies with npm and not building scaleApp and plugins, and after copying the outputs in a working project.
I understand your remark about the "size" of the project.

Managing modules dependancies with npm

you can use npm: npm i --save scaleapp  :)

I know ;)
But using the plugins like that could be nice too:
npm i --save scaleapp
npm i --save scaleapp-i18n

But using the plugins like that could be nice too

you can. e.g. require("scaleapp/dist/plugins/scaleApp.i18n")

if you like, we could create a separate plugin repo, then you'd use it like
npm i --save scaleapp
npm i --save scaleapp-plugins

var sa = require("scaleapp");
var i18n = require("scaleapp-plugins/i18n");

Yes, this could be a nice intermediate solution

so in other words, instead of - we could also use / which leads to require("scaleapp/plugins/i18n");`  which doesn't require a separate repo ;-)
And as long there is only one repo you don't have to care about versions that might be incompatible :)


Fix GpuMat::setTo implementation

Previous implementation was not thread/stream safe, since it used constant memory.
The new implementation doesn't use any global objects, so it is thread/stream safe.

:+1:


ARROW-12717: [C++][Python] Add find_substring kernel

This adds a very simple lfind kernel. If the substring is not found, -1 is reported. Nulls are propagated. Regexes are not supported, nor is rfind.

Updated to support binary/large_binary and to output int32 for non-large types. I'm still postponing regex find/rfind for a different issue. Though, I'm not sure if we want rfind to be an option of this kernel or its own kernel.

Rebased and fixed conflicts.

@lidavidm do you intend for this to be merged before or after #10369?

Let's merge this one first, then I'll fix up the other (though it's no big deal either way).


Storage & FTP doesn't work to upload files into subdirectories

Laravel Version
9.52.15
PHP Version
8.1.10
Database Driver & Version
No response
Description
Hello,
After migrating my application from Laravel 8 to Laravel 9, my FTP upload is no longer functioning :/
It only works when I place a file in the root folder. Before, when using Laravel 8, I never encountered this problem.
For example, if I want to upload my file to /documents/document/, it doesn't work.
Here's my code:
$filepath = Storage::path($document->getRawOriginal('path'));
$hash = hash_hmac('md5', config('app.hash_ftp'), $id.$type);
$file = new File($filepath);

$path = 'documents/document/'.$hash;
$path_php = 'documents/document/'.$hash.'/test.jpg';

Storage::disk('ftp')->putFile($path, $file); // not functioning !
Storage::disk('ftp')->put($path, $file); // not functioning !
Storage::disk('ftp')->put($path, Storage::path($document->name)); // not functioning !

$conn_id = ftp_connect('ftp.oenodata.fr');
$login_result = ftp_login($conn_id, config('filesystems.disks.ftp.username'), config('filesystems.disks.ftp.password'));
ftp_pasv($conn_id, true);
$put = ftp_put($conn_id, $path_php, $filepath, FTP_ASCII); // Functioning !
ftp_close($conn_id);

As you can see, I attempted to use the native PHP function ftp_put, and it works without any issues!
Steps To Reproduce
You can try the code below, with a working ftp connection:
$filepath = Storage::path($document->getRawOriginal('path'));
$hash = hash_hmac('md5', config('app.hash_ftp'), $id.$type);
$file = new File($filepath);

Storage::disk('ftp')->putFile($path, $file); // not functioning !
Storage::disk('ftp')->put($path, $file); // not functioning !
Storage::disk('ftp')->put($path, Storage::path($document->name)); // not functioning !

$conn_id = ftp_connect('ftp.oenodata.fr');
$login_result = ftp_login($conn_id, config('filesystems.disks.ftp.username'), config('filesystems.disks.ftp.password'));
ftp_pasv($conn_id, true);
$put = ftp_put($conn_id, $path_php, $filepath, FTP_ASCII); // Functioning !
ftp_close($conn_id);

Hey there,
Unfortunately we don't support this version anymore. Please check out our support policy on which versions we are currently supporting. Can you please try to upgrade to the latest version and see if your problem persists? If so, please open up a new issue and we'll help you out.
Thanks!


Custom domain name + supplied certificate for API / UI

Currently the TRE UI / API is accessed via https://tre-id.location.cloudapp.azure.com - but some customers need to offer a more branded and custom URL and associated certificate, such as https://companytre.net .
A custom domain name and key vault location of the certificate should be optionally passed in at deploy time and the terraform should configure the app gateway appropriately.
Out of scope:

Certificate rolling / renewal - the cert will be 'externally' managed, and only referenced from the TRE. When a certificate is updated or changed, the reference will be updated and a TRE deploy will need to run to reconfigure.

@damoodamoo I believe we had an issue similar to this which we had closed as we viewed the custom domain name, dns (cname) and respective certificate to be managed outside the TRE. We have also confirmed such a configuration works and easily configured.

would the configuration not be overwritten on the next deploy, if was not part of the terraform?

The only relevant configuration managed by terraform is the certificate appgw uses, but currently defined to ignore changes. So no, everything will be left as is.

With the UI fix, we have implemented this outside of the TRE - was just important to make sure the certificate is named cert-primary to avoid TF updating it on the next deploy.


third party caveats produce invalid JSON when serialized

It seems that the vid field in third party caveats is not correctly encoded, which leads to an exception when trying to serialize macaroons in JSON format.
>>> m = macaroons.create("my location", "my secret key", "my identifier")
>>> m = m.add_third_party_caveat("tp location", "tp root key", "tp caveat")
>>> m.serialize(format='json')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "bindings/python/macaroons.pyx", line 181, in macaroons.Macaroon.serialize (bindings/python/macaroons.c:2259)
    return bytes(data[:data_sz]).decode('utf8')
UnicodeDecodeError: 'utf8' codec can't decode byte 0x9c in position 90: invalid start byte

I added a print statement before the exception was raised, and it printed this:
('got serialized data', '{"v":2,"l":"my location","i":"my identifier","c":[{"i":"tp caveat","l":"tp location","v":"\x9cJ\xb4jr\x11\xa5\x13\x8e\xba\x13\xe2y\xb4\x829n\x96\x0c1b\x9e\x13\xff\x8a\x89\x10B\xe2\xff+\xf7hh\xe3W\xef\xd3SO\xe0i\x062\xcf\x93\xb1\xc3\xd8r\xa1\xf5\xb5m\\g\xad\xf1\x96\xa4\x164U\xde\t\xb0\x89\x15\xb7\x12"}],"s64":"FodXLpedS2YbvCiUOqNbArA2Edw4Plc7SBFPNHc9gBI"}')

It looks like the vid field should have been base64 encoded and then the v64 field used.

Yes.  As I mentioned in #47, the JSON format wasn't complete w.r.t. UTF-8 and supporting it well.  I think at one point I required a build flag stating it was experimental, but may have removed that.
There either is or needs to be a {,de}serialization test for unicode, third party, and strings that must be normalized when converted to unicode.


Is DAB Live down?

I suddenly stopped receiving all data into HA from my Easybox mini 3 on 11:54pm on July 29th here in Victoria, Australia.
In trying to fix it today, I've realised that I can't log into DAB Live on my phone at all any more. I've tried multiple password resets, but each time that I try to log in it just hangs on the loading wheel for 1-2 minutes before saying "Wrong email or password".

I'm also experiencing the same issue with DAB Live.
Did a fresh install on HA but cannot get past login step.
DConnect does seem to work on phone though.

Sorry for my late response, was away for a couple of days.
If the DAB Live app and/or website are not accessable, then that indicates it is a problem on the DAB backend servers side. In such a case the integration also will not able to refresh its data since it connects in the exact same way as the DAB Live app does. The integration will aslo not be able to do a fresh install, since that requires a login to the backend servers as well.
Did a quick test today on the integration using a DAB Live account that a friendly user of the integration shared with me. All seems to work fine for that account/installation.
@Blake-James , @JHAvanGestel   do you still have the issues you had a few days ago, or has it already been resolved on the DAB backend server side?

@ankohanse thanks for looking into this. I just tried logging into DAB Live and also tried DConnect on my phone and still no luck with either of them.

Works for me (fresh install)

@Blake-James  is your DAB Live account still causing issues to login into the app and the integration?
Have you tried to ask DAB Pumps if something is wrong with your account?

@ankohanse yeah I'm still having the same issue unfortunately. I submitted support requests to multiple locations online with DAB, including the form at the bottom of this page https://internetofpumps.com/private-users/
and I also tried going through my local supplier, who tried to contact DAB too but haven't been able to get any response.

 Only workaround I can think of is to try to create a new DAB Live account using another email account (if you got another one). And then to link your pump to that new account. But I hope that DAB Pumps will react and resolve things for you before you need to revert to such alternative measures.

Close due to 3 months of inactivity

I've finally fixed this after months of trying to get support from DAB. Eventually I got through to the distributer Whites International who after a few weeks have said:
"Our IoT and IT Team have been working on it and it seems the account was locked due to too many incorrect login attempts. The account has now been unlocked so you should be able to access."
What a mess for such a simple issue.


tests.vim: init (moved from vim-utils.nix)

Motivation for this change
Remove hardcoded defaults (especially as it may be problematic):
https://github.com/NixOS/nixpkgs/issues/118953
I would also like to make it possible to avoid wrriting the init.vim / vimrc file when the config is empty see https://github.com/nix-community/home-manager/issues/1907
Ideally I would like to write more tests for the vimrc generators but this would require a home-manager -like infrastructure with functions like assertFileRegex to detect that the generated vimrc contains a path to a folder with vimplugins for instance.
Things done
Moved tests from pkgs/misc/vim-plugins/vim-utils.nix to pkgs/test/vim.
Also reduced the amount of generated config.

Make it possible to have an empty config when configured adequately
removed default vim config when using native packages, it could be
source of bugs see linked issues (syntax on overrides vim highlights)

you can run the tests via
$ nix-build -A tests.vim
cc @timokau @doronbehar @mjlbach

[x] Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)

[ ] NixOS
[ ] macOS
[ ] other Linux distributions

[ ] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
[ ] Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
[ ] Tested execution of all binary files (usually in ./result/bin/)
[ ] Determined the impact on package closure size (by running nix path-info -S before and after)
[ ] Ensured that relevant documentation is up to date
[x] Fits CONTRIBUTING.md.

Result of nixpkgs-review pr 119467 at 1be8e91a run on x86_64-linux 1

4 packages built successfully:

gnvim
neovim-qt
nvimpager
spacevim

Things to watch out for:

if you set configure.beforePlugins yourself, you will need to add set nocompatible too not to lose it
filetype indent plugin on | syn on is not enabled by default for the vim-plug installer: one should not override the default of vim, at least not make it mandatory. Also sometimes it's enabled before plugins (pathogen etc,) which is not consistent


Updated CSS tutorial

Proposed CSS tutorials
Pull Request Template
Please ensure all items below are checked before creating a pull request:

[x] My changes were made in the resources folder, not in the auto-generated README.md file
[x] My submission is formatted according to the guidelines in the contributing guide
[x] My submission is ordered alphabetically based on the resource name
[ ] My submission has a useful description
[ ] I have searched the repository for any relevant issues or pull requests

Closed for inactivity.


Keep the original onload and onerror functions from script tags

Fix for #132
Right now postscribe is creating new onload and onerror events to javascript tags and overwriting if the script was written as <script src="" onload="function(){}" onerror="function(){}"></script>

We believe this is fixed in 2.0.0.  Please retest against 2.0.0 release and open a new issue following Issue Reporting guidelines in 2.0.0 if you believe this is still an issue.


Scoring on the web

The scores editing through the web is not working properly, it goes back to the old scores that are tracked by the code when I do changes through the web (nothing updated/saved).

Hi @BanaIbrahim can you write the steps to recreate this issue with some screenshots?

When I refresh or quit and re-enter the request page it returned back to the previous score, no update.
https://github.com/MagnivOrg/prompt-layer-library/assets/66751604/bc97faa9-64ee-4047-b4d8-8f4a23d60410

@BanaIbrahim I can't seem to recreate this -- what browser are you using?

Chrome (latest V), I tried to edit the score and refresh the page twice and it worked. I don't know why it needs to be set more than one to get updated?


Deprecate non-working kube-inject options

The documentation of istioctl kube-inject at https://preliminary.istio.io/docs/reference/commands/istioctl/#istioctl-kube-inject includes several options that don't work.
This PR deprecates those options so they don't appear in the documentation.  They haven't worked for a while, and probably should be removed in addition to being deprecated, but I don't want to break scripts now.
For https://github.com/istio/istio/issues/12078

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: esnible
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

istioctl/OWNERS [esnible]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@esnible: The following test failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-integ-k8s-tests.sh
50567fe03eb99a2ef5fc7ce0c1a72a45e4b86546
link
/test istio-integ-k8s-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@esnible: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-integ-k8s-tests.sh
50567fe03eb99a2ef5fc7ce0c1a72a45e4b86546
link
/test istio-integ-k8s-tests

prow/istio-pilot-multicluster-e2e.sh
50567fe03eb99a2ef5fc7ce0c1a72a45e4b86546
link
/test istio-pilot-multicluster-e2e

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

/retest

@ostromart or @ayj Can you review this?

Adding to 1.1 milestone because today is documentation day and by marking 11 non-working options as deprecated will remove them from the documentation.

ISTIOCTL_USE_BUILTIN_DEFAULTS should also be removed from the top-level makefile
$ grep -rn ISTIOCTL_USE_BUILTIN_DEFAULTS Makefile
134:export ISTIOCTL_USE_BUILTIN_DEFAULTS=1

cc @incfly who added the app probe rewrite flags in https://github.com/istio/istio/pull/10344.

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: esnible
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: linsun
If they are not already assigned, you can assign the PR to them by writing /assign @linsun in a comment when ready.
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@esnible: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-pilot-multicluster-e2e.sh
50567fe03eb99a2ef5fc7ce0c1a72a45e4b86546
link
/test istio-pilot-multicluster-e2e

prow/istio-integ-k8s-tests.sh
90698cf73fc23dedf33341aff4f1798c666c3533
link
/test istio-integ-k8s-tests

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@esnible: The following tests failed, say /retest to rerun them all:

Test name
Commit
Details
Rerun command

prow/istio-integ-k8s-tests.sh
90698cf73fc23dedf33341aff4f1798c666c3533
link
/test istio-integ-k8s-tests

prow/istio-pilot-multicluster-e2e.sh
90698cf73fc23dedf33341aff4f1798c666c3533
link
/test istio-pilot-multicluster-e2e

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@esnible is your goal to fully deprecate the parameters used to render sidecar inject template?
@mathspanda has a relevant PR, https://github.com/istio/istio/pull/10830

@incfly I wasn't aware of that PR.  The two PRs will conflict.  I will abandon my PR if this PR is likely to merge.
My only intention with this PR is to make sure the auto-generated documentation matches the implementation.  We can merge the other PR, then I will repeat my testing of all of the documented istioctl kube-inject options and create a new PR to deprecate any that no longer work.

Thanks for clarification!
I think some flags are still working, at least for example for rewriteHTTPAppProbe. They are used to render the injector template we hardcode in go code... That PR is to remove that. And thus will make work here a lot of easier.
However, that PR currently is targeting against master.
/cc @mandarjog to chime in.

@incfly Let me explain why I believe --rewriteAppProbe doesn't work.
The test case for it is pilot/pkg/kube/inject/inject_test.go and the first YAML file that test uses is pilot/pkg/kube/inject/testdata/webhook/hello-probes.yaml.  That is what I used interactively.  If that file isn't compatible with the flag let me know.
I did
istioctl kube-inject --rewriteAppProbe=false -f pilot/pkg/kube/inject/testdata/webhook/hello-probes.yaml > /tmp/norewrite.yaml
istioctl kube-inject --rewriteAppProbe=true -f pilot/pkg/kube/inject/testdata/webhook/hello-probes.yaml > /tmp/rewrite.yaml
diff /tmp/norewrite.yaml /tmp/rewrite.yaml

I saw no difference so I concluded the CLI option is not hooked up using the default configmap.  It is possible it works with some ancient configmap, but I don't see a way to get it working with the configmap used in Istio 1.1.

@esnible to see the diff several flags need to be turned on to trigger our hardcoded template. And the testing probe files should be testdata/inject/app_probe/hello-probes.yaml.
However, I totally support to deprecate the client side template rendering for istioctl kube-inject. Go for it. @mathspanda PR will further improve by removing ISTIOCTL_USE_BUILTIN_DEFAULTS=true option at all.
 ISTIOCTL_USE_BUILTIN_DEFAULTS=true /usr/local/google/home/jianfeih/workspace/golang/out/linux_amd64/release/istioctl kube-inject --hub="a" --tag="b"  --rewriteAppProbe=true -f pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml > /tmp/rewrite.yaml
ISTIOCTL_USE_BUILTIN_DEFAULTS=true /usr/local/google/home/jianfeih/workspace/golang/out/linux_amd64/release/istioctl kube-inject --hub="a" --tag="b"  --rewriteAppProbe=false -f pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml > /tmp/norewrite.yaml
diff /tmp/rewrite.yaml /tmp/norewrite.yaml

/lgtm

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: esnible, incfly
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: linsun
If they are not already assigned, you can assign the PR to them by writing /assign @linsun in a comment when ready.
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mathspanda @ayj Should we close this PR and merge https://github.com/istio/istio/pull/10830 ?
Or merge this PR and rebase 10830?

Either way, IMO the deprecated flags should be removed altogether. They aren't implemented at all. Leaving them as deprecated implies that they are still usable in some form.

@ayj Would you prefer this PR to remove the non-working options?  The only reason I deprecated them instead of removing them was to keep scripts from breaking.  If a PR that removes them without a deprecation period can be /approved I will happily revise this PR.

That would be my recommendation. I made a similar suggestion in #10830 (see here).

If a PR that removes them without a deprecation period can be /approved I will happily revise this PR.

First, many of these flags have been marked as deprecated for 10 months. Second, I don't think removing the feature support but leaving a dummy flag adheres to the intent of a deprecation period :) Any scripts that rely on these flags should be updated first (and/or in this PR) before the functionality is removed.

Closing because https://github.com/istio/istio/pull/10830 merged with similar functionality.


staticwebapp config: allow unsafe-eval and unsafe-inline in CSP

Currently the website complains that both unsafe-eval and unsafe-inline are not allowed by the Content Security Policy. However, it seems that the former is required for the search feature and the latter to be able to store the theme selection. The theme selection could be imported differently to prevent the need for unsafe-inline, but for the time being this should solve both issues.
Fixes #61

Ah, there was already #74 with a similar solution, sorry.

@flyth I merged the other one following your comments. Do you think we can close this or is there anything we need to do?

@flyth I merged the other one following your comments. Do you think we can close this or is there anything we need to do?

I think this would have given a slightly better rating because it still keeps the CSP for third party websites, but I don't think there's a big difference with the content we serve.
/cc @sniok WDYT?


texturepacker: init at 7.1.0

Description of changes
Closes https://github.com/NixOS/nixpkgs/issues/81362
This PR adds 1 package: texturepacker
This software is proprietary
To get it to work, I used the published .deb package, removed most of the libraries packaged together with it.
There were a few exceptions: two, which were not in nixpkgs, and one which, if replaced by the nixpkgs version of the lib, would not work

Things done

Built on platform(s)

[x] 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
[x] Tested basic functionality of all binary files (usually in ./result/bin/)
23.11 Release Notes (or backporting 23.05 Release notes)

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

[x] Fits CONTRIBUTING.md.

Thank you!


Get up to speed on Security Automation in general

Describe the issue
Talk to Warren about the various Security Automation approaches (Guardrails, Cloud Custodian).  Addressing Learning Debt within the team.
Which Sprint Goal is this issue related to?

Note: 'Milestone' is a ZenHub term that we use synonymously with 'Sprint'.

The 'Milestone' description (created in ZenHub) should clearly list Sprint Goals. This section should indicate which 'goal' this issue is related to.
Additional context
Add any other context, attachments or screenshots
Definition of done

Talk to Warren or Max for examples of dev setup scripts (there should be some in the Demo Apps)
Set up your Security Developer environment and create scripts that you can check in to GitHub for other people to reuse when they onboard as security developers.
Gain access and settings to be able to run the Security Automation scripts and be able to start developing comfortably.

Depends on the outcomes of the hackathon evaluating out of the box commercial multi-cloud security solutions


GeoServer - Store high-resolution HydroLAKES routing data

@huard @Zeitsperre Can we do this now, or do we need to wait for @dustming's documented codes? Is that all part of the same package?

The product and the light version GIS toolbox are linked. The lake river routing product needs an update to support the light version of the QGIS toolbox.  I am working on finalizing the light version GIS routing toolbox and document the code.  After that, I will update the routing product to make it support the light version GIS routing toolbox.

WFS layers for the HydroLAKES polygons and pour points are now available on the GeoServer.


Fix travis

Which was error?
With this travis always run examples on gtk master, so don't check changes

Yes but otherwise we have this error:
error: native library `cairo` is being linked to by more than one package, and can only be linked to by one package

For every lib. I still wonder how to overpass. :-/

You right, cargo don't allow mix crates and git= version, I still don't found way to fix it locally
error: native library `cairo` is being linked to by more than one package, and can only be linked to by one package

  cairo-sys-rs v0.3.2 (https://github.com/gtk-rs/cairo#37392b30)
  cairo-sys-rs v0.3.2

Seems cargo checks before overriding

I thought it was regression but seems last nightly works for me too in this case:
rustc --version
#rustc 1.15.0-nightly (2217bd771 2016-11-25)
cargo --version
#cargo 0.16.0-nightly (b26d672 2016-11-25)
git clone https://github.com/gtk-rs/gtk.git
cd gtk
mkdir .cargo
echo 'paths = ["."]' > .cargo/config
git clone -q --depth 50 -b pending https://github.com/gtk-rs/examples _examples
cd _examples
cargo build --features

Don't understand, why it not work.
Only difference that I doing it manually on windows and travis has 2016-11-24.

Checked with archived 2016-11-24 it works too :cry:

@GuillaumeGomez, can you change file back and run build
https://travis-ci.org/gtk-rs/gtk/jobs/179211820 where cairo error is for crates branch not master.

Restarting on master branch.

https://travis-ci.org/gtk-rs/gtk/builds/179206097

Travis says that master working good, so this PR unneeded.

Ok!


Password validation in the first step

Describe your problem(s)
When adding a User via the Carbon console, there is no validation for the password field when you click "Next". However, at the end, when clicking "Finish", the errors in the password is validated.

Describe your solution
Validate password when clicking "Next". Or provide a hint.

Currently on-hold this if we consider this once theres any requirement.


Use GCP in private_endpoint_services_resource test for dedicated

There are less limitations on GCP hosts so we prefer to use those for spinning up dedicated tests where possible.  This updates the the dedicated test in the private_endpoint_services_resource test to use GCP.
Commit checklist

[ ] Changelog
[ ] Doc gen (make generate)
[ ] Integration test(s)
[ ] Acceptance test(s)
[ ] Example(s)

The tests passed in staging.  I'm going to go ahead and merge this down and tag for the release.


Reorganise routes

move staff-facing routes under /dashboard, so that residents don't accidentally get a log in page any more if they visit the root url, and so we can properly build the staff-facing /resident routes.
update cypress tests

LGTM, would the index page just 404 now?

yes - probably less confusing than showing a log in page.

LGTM, would the index page just 404 now?

yes - probably less confusing than showing a log in page.


Add known types and candidate types

This PR is adding the KnownTypes  as a way to easily check which packages types are officially supported/present in the spec. Besides, I've added the CandidateTypes for holding those packages that are considered to be included (more info).
The rationale behind this PR is described at #59.
Note that TypeJulia was moved to CandidateTypes (even if it was priorly defined in the repo) because it does not appear in the official spec yet.
Requires #60 to be merged beforehand (as some additional types were added).
Closes #59

@antgamdia  I did a mistake during merging :/ and now I have apparently no permission to push to your branch. Can you fix the redundant TypeJulia and maybe squash the commits? Thx

@shibumi, done! Thank you for approving and merging the PRs!

@antgamdia  can you rebase? There seem to be a conflict.

Sure, will do it... though Github says This branch has no conflicts with the base branch, dunno :S

Looks like it just needed a merge instead of a rebase. Weird

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Warning: This coverage report may be inaccurate.
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 0   changed or added relevant lines in 0 files are covered.
30 unchanged lines in 1 file lost coverage.
Overall coverage remained the same at 85.022%

Files with Coverage Reduction
New Missed Lines
%

packageurl.go
30
85.02%

Totals

Change from base Build<PHONE_NUMBER>:
0.0%

Covered Lines:
193

Relevant Lines:
227

💛  - Coveralls


Improve message about device

Can we have something along those lines somewhere on the top of the start page explaining what is PSLab:
PSLab is a hardware extension for your smartphone or PC. It enables you to measure all kinds of things by connecting it to your phone or PC via USB. You can use our Android or desktop app to collect and view the data. And, you can even plug in hundreds of compatible sensors to the PSLab pin slots. What experiments you can do is just limited to your imagination!

Updated the text


Added FNMS onPrem (on the internet) support

Description
Added FNMS onPrem (on the internet) support
Issues Resolved
New feature
Contribution Check List

[x ] All tests pass.
[x] New functionality includes testing.
[x] New functionality has been documented in the README if applicable
[x] New functionality has been documented in CHANGELOG.MD

          3 Errors

:no_entry_sign:
operational/fnms/schedule_fnms_reports/CHANGELOG.md:1: MD002 First header should be a top level header
operational/fnms/schedule_fnms_reports/CHANGELOG.md:1: MD022 Headers should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/CHANGELOG.md:5: MD022 Headers should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/CHANGELOG.md:9: MD022 Headers should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/CHANGELOG.md:13: MD022 Headers should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/CHANGELOG.md:3: MD032 Lists should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/CHANGELOG.md:7: MD032 Lists should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/CHANGELOG.md:11: MD032 Lists should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/CHANGELOG.md:15: MD032 Lists should be surrounded by blank lines
A detailed description of the rules is available at https://github.com/markdownlint/markdownlint/blob/master/docs/RULES.md

:no_entry_sign:
operational/fnms/schedule_fnms_reports/README.md:1: MD002 First header should be a top level header
operational/fnms/schedule_fnms_reports/README.md:17: MD007 Unordered list indentation
operational/fnms/schedule_fnms_reports/README.md:18: MD007 Unordered list indentation
operational/fnms/schedule_fnms_reports/README.md:38: MD009 Trailing spaces
operational/fnms/schedule_fnms_reports/README.md:57: MD009 Trailing spaces
operational/fnms/schedule_fnms_reports/README.md:73: MD009 Trailing spaces
operational/fnms/schedule_fnms_reports/README.md:3: MD022 Headers should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/README.md:14: MD022 Headers should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/README.md:69: MD022 Headers should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/README.md:72: MD022 Headers should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/README.md:78: MD022 Headers should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/README.md:26: MD026 Trailing punctuation in header
operational/fnms/schedule_fnms_reports/README.md:11: MD032 Lists should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/README.md:15: MD032 Lists should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/README.md:28: MD032 Lists should be surrounded by blank lines
operational/fnms/schedule_fnms_reports/README.md:64: MD034 Bare URL used
A detailed description of the rules is available at https://github.com/markdownlint/markdownlint/blob/master/docs/RULES.md

:no_entry_sign:
Please add labels to this Pull Request

  Generated by :no_entry_sign: Danger


 U: .

 U: .

Console output
https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3989/1/console

versions
git=2.17.1 maven=3.6.3

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

This message was automatically generated.

:confetti_ball: +1 overall

Vote
Subsystem
Runtime
Comment

+0 :ok:
reexec
1m 29s
Docker mode activated.

_ Prechecks _

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

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

_ master Compile Tests _

+1 :green_heart:
mvninstall
4m 41s
master passed

+0 :ok:
refguide
4m  7s
branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.

_ Patch Compile Tests _

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

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

+0 :ok:
refguide
3m 42s
patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.

_ Other Tests _

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

19m 54s

Subsystem
Report/Notes

Docker
ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3989/1/artifact/yetus-general-check/output/Dockerfile

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

Optional Tests
dupname asflicense refguide

uname
Linux 04d4ec215fcd 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Build tool
maven

Personality
dev-support/hbase-personality.sh

git revision
master / 8f0c2dabbb

refguide
https://nightlies.apache.org/hbase/HBase/HBase-PreCommit-GitHub-PR/PR-3989/1/yetus-general-check/output/branch-site/book.html

refguide
https://nightlies.apache.org/hbase/HBase/HBase-PreCommit-GitHub-PR/PR-3989/1/yetus-general-check/output/patch-site/book.html

Max. process+thread count
65 (vs. ulimit of 30000)

modules
C: . U: .

Console output
https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3989/1/console

versions
git=2.17.1 maven=3.6.3

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

This message was automatically generated.


THRIFT-5139/THRIFT-4181: Python3 type hints

Add type hinting (with native enums (IntEnum)) for python generated code

Squashed and rebased changes from stale PR
Removed the controversial runtime type check as it goes beyond strictly annotations
Added type_hints as a default option to RunClientServer.py and generate.cmake.
Enabled testing against thrift test suite
Made enum regeneration explicitly required instead of implied when this flag is provided
Added python tests to verify basic annotations are getting added
Generate a thrift_spec attribute variable declaration for all generated thrift structures and optionally type hint it if type_hints are enabled
Note: Appveyor x86 / Python 3.5 build is failing since this PR generates variable type hints (x: int = 1), which are only python 3.6 and up. Do we have any patterns established for skipping tests based on platform/python version?

Example difference in generated code after running thrift --gen py:type_hints against:
struct OneOfEachZZ {
  1: required bool im_true,
  2: bool im_false,
  3: byte a_bite,
  4: i16 integer16,
  5: i32 integer32,
  6: i64 integer64,
  7: double double_precision,
  8: string some_characters,
  9: string zomg_unicode,
  10: optional bool what_who
}

service UnderscoreSrv {
  i64 some_rpc_call(1: string message, 2: OneOfEachZZ sample_struct)
}

ttypes.py

root@37314fef4537:/thrift/src/test# diff gen-py/FullCamelTest/ttypes.py gen-py-old/FullCamelTest/ttypes.py
6c6
< #  options string: py
---
> #  options string: py:type_hints
8a9,10
> from __future__ import annotations
> import typing
12a15
> from enum import IntEnum
37,47c40,50
<     def __init__(self, im_true = None, im_false = None, a_bite = None, integer16 = None, integer32 = None, integer64 = None, double_precision = None, some_characters = None, zomg_unicode = None, what_who = None,):
<         self.im_true = im_true
<         self.im_false = im_false
<         self.a_bite = a_bite
<         self.integer16 = integer16
<         self.integer32 = integer32
<         self.integer64 = integer64
<         self.double_precision = double_precision
<         self.some_characters = some_characters
<         self.zomg_unicode = zomg_unicode
<         self.what_who = what_who
---
>     def __init__(self, im_true: bool = None, im_false: typing.Optional[bool] = None, a_bite: typing.Optional[int] = None, integer16: typing.Optional[int] = None, integer32: typing.Optional[int] = None, integer64: typing.Optional[int] = None, double_precision: typing.Optional[float] = None, some_characters: typing.Optional[str] = None, zomg_unicode: typing.Optional[str] = None, what_who: typing.Optional[bool] = None,):
>         self.im_true: bool = im_true
>         self.im_false: typing.Optional[bool] = im_false
>         self.a_bite: typing.Optional[int] = a_bite
>         self.integer16: typing.Optional[int] = integer16
>         self.integer32: typing.Optional[int] = integer32
>         self.integer64: typing.Optional[int] = integer64
>         self.double_precision: typing.Optional[float] = double_precision
>         self.some_characters: typing.Optional[str] = some_characters
>         self.zomg_unicode: typing.Optional[str] = zomg_unicode
>         self.what_who: typing.Optional[bool] = what_who

UnderscoreSrv.py

root@37314fef4537:/thrift/src/test# diff gen-py/FullCamelTest/UnderscoreSrv.py gen-py-old/FullCamelTest/UnderscoreSrv.py
6c6
< #  options string: py
---
> #  options string: py:type_hints
8a9,10
> from __future__ import annotations
> import typing
12a15
> from enum import IntEnum
23c26
<     def some_rpc_call(self, message, sample_struct):
---
>     def some_rpc_call(self, message: str, sample_struct: OneOfEachZZ) -> int:
40c43
<     def some_rpc_call(self, message, sample_struct):
---
>     def some_rpc_call(self, message: str, sample_struct: OneOfEachZZ) -> int:
50c53
<     def send_some_rpc_call(self, message, sample_struct):
---
>     def send_some_rpc_call(self, message: str, sample_struct: OneOfEachZZ):
59c62
<     def recv_some_rpc_call(self):
---
>     def recv_some_rpc_call(self) -> int:
137,139c140,142
<     def __init__(self, message = None, sample_struct = None,):
<         self.message = message
<         self.sample_struct = sample_struct
---
>     def __init__(self, message: typing.Optional[str] = None, sample_struct: typing.Optional[OneOfEachZZ] = None,):
>         self.message: typing.Optional[str] = message
>         self.sample_struct: typing.Optional[OneOfEachZZ] = sample_struct
212,213c215,216
<     def __init__(self, success = None,):
<         self.success = success
---
>     def __init__(self, success: typing.Optional[int] = None,):
>         self.success: typing.Optional[int] = success

Unrelated to these changes, I noticed that even if a field is marked as required, declare_argument generates a default value in the function arguments of None.

Open questions: Since the original PR opened,

[ ] Did you create an Apache Jira ticket?  (Request account here, not required for trivial changes)
[x] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
[x] Did you squash your changes to a single commit?  (not required, but preferred)
[x] Did you do your best to avoid breaking changes?  If one was needed, did you label the Jira ticket with "Breaking-Change"?
[ ] If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

It would be great to have this, it would almost enable transpilation via mypyc. I believe the only missing piece would be to add to all structures:
thrift_spec: Any

@arkuhn According to ASF rules you cannot simply talke other peoples code and contribute it., The original author (Konstantin Pozdniakov) can.

@arkuhn According to ASF rules you cannot simply take other peoples code and contribute it., The original author (Konstantin Pozdniakov) can contribute himself.

I see @Jens-G. Does this mean we can not add type hints for thrift generated python libs until the original author returns? The original author hasn't commented on the original PR in 2 years.
Similarly, if he does return can he include the changes I've made (mostly testing changes, but also functional changes) in his branch or is that prohibited too?
I could rebase his commit on top of lastest main in this branch so his authorship still is credited in the commit log, or I could change the implementation to functionally do the same thing but differently. I'd mostly just like to see the functionality get added.

@cspwizard Have you read this?

@cspwizard Have you read this?

Hi! Now I did. Feel free to use my code. If there is anything  I can do to help - let me know.
There was an issue with my PR about duck typing. I've added some constraints to check the type of the complex property, to prevent garbage (something not defined by the contract) in the serialized message and we couldn't come to agreement on that with maintainers.

So it seems we have permission from the original contributor, as well as @arkuhn 's changes having its comments addressed.
Is all that is missing that this PR be rebased? What can I do to help get this across the line?


Get QEMU logs from virtlogd

What this PR does / why we need it:

It reads the logs from /var/log/libvirt/qemu/domain.log (which is handled by virtlogd), structuralizes it and log them with  kubevirt.io/client-go/log so it is reachable outside the pod

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2953
Special notes for your reviewer:
This is a WIP, I want to double check that this is the right direction. I'm just reading from the file created by virtlogd for now.
Release note:

QEMU logs can now be fetched from outside the pod

CC @enp0s3

Besides adding a unit test, I changed the subcomponent to "qemu" as suggested by @enp0s3 and renamed the log function to reflect that.

/lgtm

Let's see what the tests say about the change. @victortoso thanks.

/approve


New Vertica 7.x driver

Hello,
It would be great if you add in the rope the version 7.X of the vertica drivers because there are possible connection problem between 6.x drivers and 7.x servers.
You can find them here https://my.vertica.com/download/vertica/client-drivers/ while an article useful for this issue is this: https://community.dev.hpe.com/t5/Vertica-Forum/Occassional-Error-during-COPY-statement-using-Python-Pyodbc/td-p/205089 which does not talk about the dbeaver usage but we have between some connections.
Please consider this as very useful for us.
Thanks in advance.
SR

That's not so easy.

As far as I know there is backward compatibility problem with JDBC drivers (i.e. you can't use new driver with old database - at least this problem was there for 5 and 6 versions).
Vertica drivers have proprietary license which do not allow drivers redistribution. Similarly to Oracle and DB2 drivers.

So for now the best solution is to download appropriate drivers from Vertica website.

Thanks a lot.
We will put it manually as the driver.
SR


Laravel 11 support

Can't really install the package in Laravel 11, would provide a PR, but don't really know what needs to be changed to make it installable and I don't know what to change to make it compatible with Laravel 9,10 and 11 at the same time.
This is the error message from composer:
 Problem 1
    - nathanheffley/laravel-watermelon[v0.0.1, ..., v0.0.2] require illuminate/database ^8.0 -> found illuminate/database[v8.0.0, ..., v8.83.27] but these were not loaded, likely because it conflicts with another require.
    - nathanheffley/laravel-watermelon[v0.0.3, v1.0.0] require illuminate/database ^10.13.2 -> found illuminate/database[v10.13.2, ..., v10.48.4] but these were not loaded, likely because it conflicts with another require.
    - Root composer.json requires nathanheffley/laravel-watermelon * -> satisfiable by nathanheffley/laravel-watermelon[v0.0.1, v0.0.2, v0.0.3, v1.0.0].

Please try using v2.0.0. It is installable so I will close this one, but please open another issue if you run into any bugs!


Codelabs css / js is broken

All our codelabs linked from https://bumble-tech.github.io/appyx/how-to-use-appyx/codelabs/ started rendering as plain html files.

Seems to be on Google's side they removed these assets:
https://groups.google.com/g/codelab-authors/c/WwSdRF4bjBE

@zsoltk / @vladcipariu91 it seems to be fixed

We solved it by self-hosting those files as google suggested in the above linked thread, so we don't depend on them.


Support variable sets

One microservice is one Github repository under main branch. ( we have 10 microservices = 10 Github repositories
30 AWS accounts where we deploy our microservices. ( that's 15 customers where we deploy 2 AWS accounts per customer, sandbox and prod
Based on that, it's a complex setup where we need to maintain and deploy 10 microservices per AWS account, but each AWS account microservice has different variables.

Closing in favor of #795


Extracted popOver widget definition from popOver.js and placed in pop…

…Over.def.js
Created popOver.test.js test file based on popOver.spec.js jasmine tests

The problem is, those tests are testing show/hide which is a slow process -  it has a fadeIn/fadeOut that slows down the process

We will need to profile the hide(), show() and position() methods to see how to speed them up.

I made some improvements.  There are still two tests that are slower than they should be, but it's better.
     0ms : can hide itself
     8ms : can set its position around its target```


Sixel output messed

Describe the bug
When i try to press an keychord such as g it shows the shortcut menu below Playback section.
This leads to the sixel image preview getting broken. When the keychord is completed , the playback section goes to its original position but sixel isn't redrawn.
I can make it redraw the image forcefully by resizing the terminal.
To Reproduce

have sixel enabled
play a song
press a keychord (e.g. g l)
the image preview is broken

Expected behaviour
I believe the solution could be either fixed by making the shortcut menu appear above the playback like pre 0.17.0 or making the screen redraw the image when it is changed
Screenshots
If applicable, add screenshots to help explain your problem.

Environment

OS: Nix OS(Linux)
Application version: 0.17.0
Application features: sixel

Should be fixed by #390. Can you try running from that branch?

It does redraw but leaves some artifacts and the text from before isn't cleared

 It does redraw but leaves some artifacts and the text from before isn't cleared

I don't have this problem for iterm2. It might happen because sixel doesn't render the image fully in the allocated area. Can you try to tweak cover_img_width, cover_img_length, cover_img_scale to see if those configs fix the issue.
Personally, I don't want to revert to the old popup behaviour in which popups are rendered above the playback (if in bottom) as it looks weird.

I tried a number of scaling options. But the text is still under the image. Maybe if there is a way to replace the section under image with spaces just like how the rest of playback is overrides the popup text

Scaling can be set to be >1 FYI, so you can make the image to cover the whole section with text artifact. Can also change length/width of the image rectangle to achieve the best-looking result. The default settings are not tuned for sixel at all as I don't really that feature.
If none of the above options works, I can take another look this weekend to see if there is a better way to fix the underlying issue.

@71zenith figured out a better way to handle the re-draw. Can you try again with the latest change in https://github.com/aome510/spotify-player/pull/390?

I tried it but the problem still remains

I tried it but the problem still remains

Hmm super weird. That means Clear doesn’t clear the area.

Can you share the configs you are using now, e.g what is the width/length of the cover image rectangle?

@71zenith , I managed to add some hacks to reproduce your issue with iterm2. Pushed a fix, which I strongly believe should fix the issue.

border_type = "Hidden"
client_id = ""
client_port = 8080
cover_img_scale = 2.4
default_device = ""
enable_media_control = true
liked_icon = ""
pause_icon = " "
play_icon = " "
playback_window_position = "Bottom"
progress_bar_type = "Rectangle"
theme = "oxocarbon"
track_table_item_max_len = 32
tracks_playback_limit = 5

[device]
audio_cache = false
bitrate = 320
device_type = "speaker"
name = ""

@71zenith , I managed to add some hacks to reproduce your issue with iterm2. Pushed a fix, which I strongly believe should fix the issue.

Sadly it still doesnt fix it.

Does it still have text under the image or different issue with image not fully rendered, which I'm aware of and trying to fix now?

My last attempt in https://github.com/aome510/spotify-player/pull/390/commits/4f28c2e54056cc1f8ac0185f2d6802386a391359, if this still does not work. I'll revert to the old behaviour (popup above playback for bottom) then 😞 .

Does it still have text under the image or different issue with image not fully rendered, which I'm aware of and trying to fix now?

Image is rendered correctly but it still has text below it

Does it still have text under the image or different issue with image not fully rendered, which I'm aware of and trying to fix now?

Image is rendered correctly but it still has text below it

Are u using the latest version of https://github.com/aome510/spotify-player/pull/390?

Still don't understand why my latest patch didn't work for you 😕. That said, I reverted the rendering order back to before v0.17.0 so it should work now. Thank you for all the back-and-forth!

@aome510 i am really really sorry. it looks like i did a build for release and was testing the binary for debug. It works perfectly with your last patch without any artifacts. I should have tested more clearly

No worries. Glad to hear that I'm not hallucinating. I can push a "revert revert" change xD


Generate virtual custom attributes with sections

pulled out from https://github.com/ManageIQ/manageiq/pull/14823
VCA = Virtual Custom Attributes
When there are more custom_attributes with same name we were not able to distinguish them.
In this PR I distinguish them by column CustomAttribute#section
and I am extending format of virtual custom attributes to
virtual_custom_attribute_NAMEOFATTR:SECTION:docker_labels
and thanks to that it has a format in UI
Docker Labels: NAMEOFATTR and
when there is no section it looks like:
Custom Attribute: NAMEOFATTR so it
means that this also works with origin form. (part :SECTION: is not mandatory)
Images
let's assume that we have 3 different custom attributes with name com.redhat.component
but with different sections
on the report:

columns in report definition

expression

Links
follow up https://github.com/ManageIQ/manageiq/pull/14391
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1434077
Depends on
https://github.com/ManageIQ/manageiq/pull/14835
It is a reason for WIP.
@miq-bot add_label enhancement,core, fine/yes, euwe/yes, wip
@miq-bot assign @gtanzillo
cc @zeari @simon3z

From @simon3z

It will work but it is strongly recommended to re-select them because then you can use #section for distinguishing. without re-selecting, we are not able to determine wich attribute is which.
I hope, this is acceptable for existing reports.

@gtanzillo I suppose this should go in the release notes and somehow reach the users. cc @Loicavenel

cc @zeari

@miq-bot remove_label wip

@simaishi This is second part for https://bugzilla.redhat.com/show_bug.cgi?id=1434077

Backported to Euwe via https://github.com/ManageIQ/manageiq/pull/14971

Fine backport details:
$ git log -1
commit 9d5c250378a30205eba621a02731b428316da793
Author: Gregg Tanzillo<EMAIL_ADDRESS>Date:   Tue Apr 25 10:07:09 2017 -0400

    Merge pull request #14837 from lpichler/generate_virtual_custom_attributes_with_sections

    Generate virtual custom attributes with sections
    (cherry picked from commit 688310e3bbc2a8e7b8d93145585f2252246c10b9)

    https://bugzilla.redhat.com/show_bug.cgi?id=1458341


Implement AVALON_LABEL

Also an initial pass of #189, Session
AVALON_LABEL is used by the menu item in the hosts. The intent is to enable studios to customise the look of it, to merge better with their studio. I'm calling it Mindbender for the Mindbender guys.
I've also made a first pass of api.session which doesn't make any functional changes, apart from collecting variables used in pipeline.py. The goal is to see whether we can reference all state from this one dictionary. At that point, we can start having a look at making so that we can instantiate a new one within the same process, so as to enable #107 and simplified testing amongst other things.

Usage
$ set AVALON_LABEL=Mindbender
$ python -m launcher

Example

I've also made a first pass of api.session which doesn't make any functional changes, apart from collecting variables used in pipeline.py.

I think currently the implementation feels off. Since it's a dictionary stored at the module level you're still not allowing multiple sessions at the same time. As such in this way it wouldn't introduce any benefits, would it?
Using sessions shouldn't two tools be able to operate on different sessions within the same python interpreter session? I'd say yes. But if not, why would you have a session?
The implementation as described in #189 makes more sense to me, where you instantiate a session object and that's what you keep around instead of the "api" holding on to it.

As such in this way it wouldn't introduce any benefits, would it?

It does not. That's what I meant with that it didn't make any changes, other than move variables around in pipeline.py. The idea is to keep expanding the areas that make use of these variables elsewhere, such as in plug-ins. But I don't think that can be done in one big go.
This is a small step in that direction, I'm expecting it to change into it's own object when the need arises and when we've refactored all references into using this object.

But if not, why would you have a session?

Short answer, baby steps.

Sure, in that sense it could be a decent intermediate. Should it also house app and task?

Sure, in that sense it could be a decent intermediate. Should it also house app and task?

Yes, let's start with this, as this PR is about LABEL.
Continue thinking about where you use state in your code from now on, and see whether you can refactor it into api.session.


Do I need to install dotnet?

Sorry if this is the wrong place to ask.
I learning about Docker these days and I like the fact that you don't have to install anything to get up and running with a new technology.
Is it possible to download the vscode extension and not having to install dotnet cli tooling on my local machine? The dotnet cli is installed within the Docker container so can vscode use it from there?

@nojaf You can use VS Code as an editor without a local CLI install, and if you are willing to do a lot more work you can get other things to work. But if you want things to be easy you will need dotnet on the path that VS Code is started from. You don't necessarily need to go through the installer, but obviously that makes things easier. Here is how dotnet is used --

Dotnet includes the ability to restore nuget packages, and that is how we acquire the debugger
While you are certainly free to build your code anyway you want, the default experience is to build your code locally, which isn't going to work without a local dotnet CLI.
This one is less required, but obviously if you want to run your code locally you will need the .NET CLI.

Ok, thanks. I'll try and get this working.
Any chance some work will be done to streamline this process?
For example a project setting where you can tell vscode what container you are using.
{
  "docker-container-name":"my-container"
}

If this setting is present package restore would try and execute:
docker exec my-container dotnet restore

@nojaf I don't know much about the plans for the VS Code Docker tooling. @SteveLasker can you comment?

Is it possible to download the vscode extension and not having to install dotnet cli tooling on my local machine

I think this is true going forward, as it will use desktop .net/embedded mono?

This issue looks pretty stale to me. I'm going to go ahead and close it. Please let me know if there are any concerns -- especially with regard to the Docker question which hasn't been answered (though it's now 6 months old).


Create a public merging endpoint that checks dataset access

This is ready to go but depends on deployment of a new docker compose, requested here: https://redmine.apidb.org/issues/50866

The functionality here was merged into the phase1-derived-vars branch and then modified (paths).  Closing this PR since it is overridden by that PR/branch.


ZF2 url view helper can't reuse query parameter

Since route "Query" is deprecated, we can't reuse query parameter:
Example: Currently my url is http://localhost/lead?query=&agentId=1&submit=Search after submited my form to ordered my list.
But if i click to sort column with url helper:
$urlHelper = $this->view->plugin('url');
$url = $urlHelper(null, [], ['query' => ['order' => $element, 'orderBy' => $by]], true);

Generated url is equal to http://localhost/lead?order=firstname&orderBy=ASC
Expected : http://localhost/lead?query=&agentId=1&submit=Search&order=firstname&orderBy=ASC
Did not find solution for this one.
Thx.

The $reuseMatchedParams is used only for route parameters and url queries are an option. I created a new view helper that reuses query parameter too:
https://github.com/Lansoweb/LosUi/blob/master/src/LosUi/View/Helper/Url.php
So i just call:
$this->losUrl()->from('/user',[],true);
$this->losUrl()->from('/user',['id'=>1],['query'=>['page'=>2]],true);

It uses the $reuseMatchedParams and applies to query parameters too.

I am looking for that functionality too. Maybe as an option, instead of the flag

I would realy like this feature too

@Lansoweb thx nice idea

@gizzm0 Glad to help!

+1 for reuse query params

This issue has been closed as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html


[Serialization] Store the path inside ModuleSearchPath as a string, not a StringRef

The idea behind storing a StringRef in https://github.com/apple/swift/pull/40155 was to reduce the memory footprint because we made the assumption that a ModuleSearchPath always outlives the SearchPathOptions it was created from. What I did not consider was that the path was referencing into a std::vector, which could get resized, thus invaliding the memory the ModuleSearchPath’s StringRef was pointing to, causing memory corruption.
To fix this, store the path string inisde the ModuleSearchPath itself. Since we store a ModuleSearchPath for every file inside that module search path in the LookupTable, by itself this would cause a new copy of the path to be stored for every file inside a module search path. To avoid this, make ModuleSearchPath ref counted and only store a reference to one shared ModuleSearchPath entry in the lookup table.
rdar://88888679

@swift-ci Please smoke test

Another way we could do this would be to have a BumpPtrAllocator that we just copy the paths into and then clear on a rebuild. IMO that would be a little cleaner (no need to mess around with the ref counted pointer), but this method works as well 🤷.

I tried the same but if we do this we need to be careful to always store strings in the bump allocator before referencing them in ModuleSearchPath. I decided that that’s just too easy to get wrong and that’s why I went with the ref-counted ModuleSearchPath. Plus it also had the benefit of de-duplicating the memory for Kind, IsSystem and Index.
We could think about storing the ModuleSearchPaths in a bump allocator, but again, I think the extra complexity of having to manage the bump allocator is not really worth the IMHO super minor performance benefit.

I tried the same but if we do this we need to be careful to always store strings in the bump allocator before referencing them in ModuleSearchPath. I decided that that’s just too easy to get wrong and that’s why I went with the ref-counted ModuleSearchPath. Plus it also had the benefit of de-duplicating the memory for Kind, IsSystem and Index.

I meant an allocator in ModuleSearchPathLookup, so it would just be the one copy where the new ModuleSearchPath is right now. But as I said, I'm happy with either. Was just mentioning in case you thought it helped 👍

Ah, OK. Still, I think I like the ref-counted ModuleSearchPath better. Merging.


Allow referring to selected formula in other parts of query without repeating formula

Is your feature request related to a problem? Please describe.
I want to be able refer to my selected formula column alias elsewhere in the query (for example in orderBy) instead of repeating the formula subquery (which is usually slow)
Describe the solution you'd like
Maybe it can detect if the formula column has been selected and then reuse its alias instead of repeating it. Or maybe we can use the  raw() and as()  helpers from v6 to define an alias in the formula defintion.
Describe alternatives you've considered
I've tried to use the query builder and select everything myself but I ran into many unrelated problems with that.
Additional context
See discussion #4540 for my specific use case.

IIRC the problem here is that in some parts you need to repeat it, e.g. in where query.

IIRC the problem here is that in some parts you need to repeat it, e.g. in where query.

I don't understand what you mean. Could you give an example?

I see.
Well we can at least support it for the clauses that evaluate after the selecting is done, such as ORDER BY. That's my main problem really. A subquery in ORDER BY is very slow and it's a shame to repeat it when it's already been selected and aliased.
Alternatively allow setting an alias for the formula explicitly maybe using  v6's raw and alias helpers. I don't know.

I don't understand how is this related to aliasing, the formula property already has a name and is aliased based on it.

I don't understand how is this related to aliasing, the formula property already has a name and is aliased based on it.

You're right. I was confused the alias is unrelated.
Anyway I think the enhancement would make formulas better.

Yeah, agreed it would be better, and it should be relatively simple.

That post also mentions that the engine can be capable of caching the expression (it mentions sql server), so would be good if you confirm this actually makes a difference perf-wise.

so would be good if you confirm this actually makes a difference perf-wise.

It makes a huge difference for me. I have something like 20,000 records in postgres db. Selecting, ordering by formula column, limit 25,the query takes 1 minute.
If I change the query to use ORDER BY formula_column instead of ORDER BY (formula subquery) it takes around 200ms.
This is probably because the formula involves a subquery, but still.

This is probably because the formula involves a subquery, but still.

Please compare the query execution directly, and try to have it as generated, and then only change the proposed things, so only replace the subquery in order by clause to use the selected alias (if its not selected, add it to the select clause).

After spending some time creating I repro I realized the performance issue was not this. I misjudged.
Specifically I ran the following queries:
SELECT ea.*,
       (SELECT COUNT(entity_b_id) FROM entity_a_entity_b_pivot WHERE entity_a_id = ea.id) AS entity_bs_count,
       entity_b.*

FROM entity_a ea
         LEFT JOIN entity_a_entity_b_pivot ON ea.id = entity_a_entity_b_pivot.entity_a_id
         LEFT JOIN entity_b ON entity_a_entity_b_pivot.entity_b_id = entity_b.id
GROUP BY ea.id, entity_b.id
ORDER BY entity_bs_count desc, ea.id
LIMIT 25;

And there was no significant difference.
I found out the performance issue I was seeing was due to something unrelated: The formula being used in the ORDER BY of the inner subquery used for pagination.
I'm still getting to the root of it, but the point is there was no performance difference and I should have tried to repro before reporting. Sorry about that.
I guess the only reason to do this now is for getting a more elegant, better looking query given that the DB engine seems to already optimize the performance. At least postgres.

Actually after a bit of investigation, I found out my problem could be solved with better indexes.
I was indexing the joint fk columns of the pivot table but then counting in the subquery based on a filter on one of them. And that column was the second in the composite index. Reversing the order of the columns in the index fixed the slowness. I'm also considering adding a single index on both columns.
I know this may be taking the easy way out and maybe I should try to write better queries that don't need to order based on a subquery in the first place but it works for me.
I'll try the select in as well see if that makes a difference but my problem is solved and it was unrelated.
Anyway you're right that this is not worth it, so feel free to lose this. And thanks for your help.