WAN download throughput issue with bxe/vlan

Important notices
Before you add a new report, we ask you kindly to acknowledge the following:
[X] I have read the contributing guide lines at https://github.com/opnsense/plugins/blob/master/CONTRIBUTING.md
[X] I have searched the existing issues and I'm convinced that mine is new.
[X] The title contains the plugin to which this issue belongs
Describe the bug
My WAN connection is FTTH and comes in through a DELL 0N20KJ  BROADCOM 57810 10GB DUAL PORT PCI-E SFP+ NETWORK CARD. This card uses the if_bxe.ko module.
When I run speedtests from a 1000BaseT connected machine on my LAN then I get 0.2mb/940mb results.
What is odd though is that if I run a speedtest on the Opnsense machine directly are as expected (1260/990). Also when I transfer a file from the Opnsense machine on the LAN to another machine on the LAN then I get expected speeds.
Another thing that is odd is that if I use the ISP supplied modem and connect it to Opnsense via 1000BaseT instead of the SFP module then I get expected results (940/940). The key difference here is that I am not using my Dell ON20KJ and am not using a VLAN (my ISP delivers internet over VLAN 35 but the modem takes care of that in this configuration).
I suspect that either this is a regression in the BXE driver or maybe an issue with having the WAN interface be a VLAN. I don't see anything in my logs to indicate an issue of any sort and have tried both an upgrade from 21.7 and a fresh install.
My setup was working as expected with the latest version of 21.7.
To Reproduce
Steps to reproduce the behavior:

Assign WAN to Dell 0N20KJ
Do a speedtest on a LAN machine

Expected behavior

Expect to get 940/940
Actually get 0.2/940

Screenshots
N/A - Can get if necessary
Relevant log files
N/A - Can get if necessary
Additional context
N/A - Can get if necessary
Environment
Opnsense 22.1
Dell 0N20KJ (Dual SFP+)
Onboard Intel i210 internet
Additional Intel iGB ethernet card
HP 600 G1 (Intel i5-4460 based)

Wrong area - didn't notice this was plugins.


Use #fileID and #filePath instead of #file

Codecov Report

Merging #348 (45b9478) into main (369a9ee) will not change coverage.
The diff coverage is 14.86%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #348   +/-   ##
=======================================
  Coverage   41.20%   41.20%
=======================================
  Files         117      117
  Lines        9657     9657
=======================================
  Hits         3979     3979
  Misses       5678     5678

Impacted Files
Coverage Δ

Sources/PostgresNIO/Data/PostgresRow.swift
49.24% <ø> (ø)

...ces/PostgresNIO/Deprecated/PostgresData+UInt.swift
0.00% <ø> (ø)

Sources/PostgresNIO/New/PostgresCell.swift
91.17% <ø> (ø)

...gresNIO/New/PostgresRowSequence-multi-decode.swift
0.00% <0.00%> (ø)

...ces/PostgresNIO/New/PostgresRow-multi-decode.swift
5.83% <10.00%> (ø)

...rces/PostgresNIO/New/Extensions/Logging+PSQL.swift
22.72% <28.57%> (ø)

...ostgresNIO/New/PostgresBackendMessageDecoder.swift
94.02% <85.71%> (ø)


I have no idea how to use this plugin

I figured out how to install it, with the Json and JS files, but have no idea how to actually use it. I tried making some meta data with --- ,but the information on the main page here doesn't really simplify the process. Just talks about WHAT it can do, but not HOW to do it.

Thanks for feedback, to use the plugin you need to (after installing it) define a table (point 2 in Example section in main page), after that you need to define some files in the directory pointed in source section. The files should have frontmatter metadata with keys matching name in fields sections, eg.
motorcycles/r1250rt.md:
---
brand: BMW
model: R1250RT
year: 2022
---

After that the table will show the formatted data :)


How can I use flutter_screenutil package for my hybrid app?

Here is how I've used flutter_screenutil in my project but getting error on when I tap on Textfield.
Kindly tell me how to use this package properly.
My error:

No Overlay widget exists above EditableText-[LabeledGlobalKey<EditableTextState>#d3506](controller: TextEditingController#dc3bb(TextEditingValue(text: ┤ghf├, selection: TextSelection.collapsed(offset: 3, affinity: TextAffinity.downstream, isDirectional: false), composing: TextRange(start: 0, end: 3))), focusNode: FocusNode#482e2([PRIMARY FOCUS]), autocorrect: false, debugLabel: ((englishLike titleMedium 2014).merge(blackMountainView titleMedium)).merge(unknown), inherit: false, color: Color(0xff878787), family: EncodeSans_regular, familyFallback: [EncodeSans], size: 16.0, weight: 400, baseline: alphabetic, decoration: TextDecoration.none, textAlign: start, keyboardType: TextInputType(name: TextInputType.text, signed: null, decimal: null), autofillHints: [], dirty, dependencies: [Directionality, MediaQuery, ScrollConfiguration], state: EditableTextState#1d73f(tickers: tracking 1 ticker)). Usually the Navigator created by WidgetsApp provides the overlay. Perhaps your app content was created above the Navigator with the WidgetsApp builder parameter. 'package:flutter/src/widgets/text_selection.dart': Failed assertion: line 610 pos 7: 'overlay != null'

My code :
main.dart
void main() async {
  await ScreenUtil.ensureScreenSize();
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      builder: (ctx, child) {
        ScreenUtil.init(ctx);
        return Theme(
            data: ThemeData(
              primarySwatch: Colors.blue,
            ),
            child: const HomeScreen());
      },
    );
  }
}

HomeScreen.dart
class HomeScreen extends StatefulWidget {
  const HomeScreen({Key? key}) : super(key: key);

  @override
  State<HomeScreen> createState() => _HomeScreenState();
}

class _HomeScreenState extends State<HomeScreen> {
  @override
  Widget build(BuildContext context) {
    ScreenUtil.init(context, designSize: const Size(360, 739));
    TextEditingController searchController = TextEditingController();

    return SafeArea(
      child: Scaffold(
        body: Padding(
          padding: EdgeInsets.symmetric(
            horizontal: kPaddingHorizontal,

          ),
          child: Column(
            children: [
              Row(
                mainAxisAlignment: MainAxisAlignment.spaceBetween,
                crossAxisAlignment: CrossAxisAlignment.center,
                children: [
                  Column(
                    crossAxisAlignment: CrossAxisAlignment.start,
                    mainAxisAlignment: MainAxisAlignment.center,
                    children: [
                      Text(
                        "Hello, Welcome",
                        style: kEncodeSansRegular.copyWith(color: kDarkBrown),
                      ),
                      SizedBox(height: 5.h),
                      Text(
                        "Hello, Welcome",
                        style: kEncodeSansBold.copyWith(color: kDarkBrown),
                      ),
                    ],
                  ),
                  CircleAvatar(
                    radius: 23.r,
                    backgroundImage: const AssetImage("assets/images/image5.png"),
                  ),
                ],
              ),
              SizedBox(height: 20.h),
              Row(children: [
                Expanded(
                  child: TextField(
                    style: kEncodeSansRegular.copyWith(color: kDarkGrey),
                    controller: searchController,
                    keyboardType: TextInputType.text,
                    autocorrect: false,
                    decoration: InputDecoration(
                      isDense: true,
                      contentPadding: EdgeInsets.symmetric(horizontal: 10.w),
                      prefixIcon: const Icon(
                        Icons.search,
                        color: kGrey,
                      ),
                      hintText: "Search Clothes...",
                      border: kInputBorder,
                      errorBorder: kInputBorder,
                      disabledBorder: kInputBorder,
                      focusedBorder: kInputBorder,
                      focusedErrorBorder: kInputBorder,
                      enabledBorder: kInputBorder,
                      hintStyle: kEncodeSansRegular.copyWith(color: kDarkGrey),
                    ),
                  ),
                ),

I can't tell if it's screenutil's problem


Resolve applying user scripts on Android on sites where trusted-types CSP is activated.

Connection with issue(s)
Resolve applying user scripts on Android on sites where trusted-types CSP is activated.

Testing and Review Notes

Try to login to gmail and you'll see Content-Security-Policy-Report-Only: require-trusted-types-for 'script'; report-uri https://csp.withgoogle.com/csp/boq-infra/googlegrowth-boq-js-css-signers header on  https://gds.google.com
Screenshots or Videos

To Do

[ ] double check the original issue to confirm it is fully satisfied
[ ] add testing notes and screenshots in PR description to help guide reviewers
[ ] request the "UX" team perform a design review (if/when applicable)

@alllohaaa why did you close this pull request? Doesn't work or is there something else? thanks

@alllohaaa why did you close this pull request? Doesn't work or is there something else? thanks

This only solve the part of the problem. Another one that scripting is prohibited at all with nonce in CSP. I need some time to investigate what we can do with this

Ok! The fact is that Android WebView doesn’t support natively the Content World feature such as we have on iOS with WKWebView, so probably we should stay with this limitation or hope on the Android WebView team to implement this feature natively.


A bug in variable merging.

KreMLin (version 4e48d26934965379b6b90363d4063dd277c04ca7) generated incorrect C code with the -fmerge aggressive options.
Low* Code
open U32

inline_for_extraction
private let test1(b: U32.t{U32.v b < 10}): Tot U32.t =
  let g = 10ul +^ b in
  if g >^ 5ul then
    let d = g -^ 1ul in
    g +^ d
  else
    0ul

let rec test2(a: U32.t{U32.v a < 10}): Tot U32.t (decreases U32.v a) =
  let c = test1 a in
  if a >^ 0ul then
    test2 (a -^ 1ul)
  else
    c

Compiled C Code
uint32_t test2(uint32_t a0)
{
  uint32_t a = a0;
  while (true)
  {
    uint32_t g = (uint32_t)10U + a;
    uint32_t c;
    if (g > (uint32_t)5U)
    {
      a = g - (uint32_t)1U;
      c = g + a;
    }
    else
    {
      c = (uint32_t)0U;
    }
    if (a > (uint32_t)0U)
    {
      a--;
    }
    else
    {
      return c;
    }
  }
  KRML_HOST_EPRINTF("KreMLin abort at %s:%d\n%s\n",
    __FILE__,
    __LINE__,
    "unreachable, returns inserted above");
  KRML_HOST_EXIT(255U);
}

The C code above will loop infinitely.

Is this using the combination of -fmerge and -ftail-calls?
Thanks for the repro. From the Changelog,
### Jan 13th, 2020

 - New *experimental* option: `-fmerge`, to merge variables, i.e. reuse existing

so, I don't think it's been extensively tested. I'd happily take a fix if you have one.


Changing our stylish-haskell options to be diff-safe(r).

Our previous settings can create really pointless merge headaches if two
people have been working on the same file.
This does create a longer files, but who cares? Whitespace is cheap,
time spent resolving needless merge conflicts is not.
(The purpose of a code formatter should be to reduce formatting
problems, not create them. :-D)

Thanks, I hate it. I mean, I think what I actually hate is explicit import lists, but this really does make the section that I need to scroll past to see anything useful much longer, so I'm not wild about it.

 :-D
How about I write you an emacs on-load hook that automatically folds the import section? :-D

Explicit import lists are a gift you give to others.

Perhaps I am a philistine but I don't find they give me much. I wouldn't object if the computer managed them, but I do object to managing them myself.
(If you want to be explicit, I much prefer import as and qualifying at the use sites.)

implicit import lists are probably the number 1 thing that I find makes reading other people's Haskell difficult. I want to know where the hell somefunction comes from.

Also, me from 1 month ago == another person

There are at least two people on every programming project. Even solo ones.

Oh and having an import as for everything is not nice, I find it much nicer to use import as for things that can be coming from more than 1 place. They can really increase verbosity and make things difficult to read if over-used.

Use tags? Or HIE? Or I usually find a grep works fine. Maybe I'm just weird but I've never had a problem with lack of explicit imports.

I often want to read a Haskell project outside of an IDE, for example PRs or just looking at different branches in github. Even if I'm in an IDE if I haven't set up HIE or tags etc it can be a real PITA just to find out which module some function comes from. If I'm in a well-setup project then HIE or whatever can usually add the import for me, I only have to be concerned about removing imports but that can be done at the end based on warnings.
I do see/appreciate the other side of the argument though, it's very much a YMMV thing but the other argument for this is of course the diffing issue. Clearly I'm all for this change but I appreciate there are definitely 2 sides to this.

I'm in favour of diff-resistant formatting, but I think that explicit imports list make this worse either way:

If you don't split them over many lines they're terrible for diffs.
If you do split them your import section becomes enormous. I didn't even look at the modules with big lists, but some of them have e.g. 15 items per import which would be just appalling.

My reaction is: so much the worse for explicit import lists :p But I actually think I prefer the diff sensitivity to the incredible import section length.
(And yes, I should probably set up auto-folding.)

I apologise in advance for the tweet I can't resist posting.

Eh, actually looking at more of the output I don't hate it as much as I initially thought. Although possibly that's due to the fact that we're not using that many explicit import lists at the moment ;)

I quite like the ormolu output. However:

I'm not wild about 2 space indenting. I find this a bit hard to read, but I think I'd get over it pretty fast.
https://github.com/tweag/ormolu/pull/394 is the only blocker that I found. I really like spacing out blocks of code in do blocks, and currently ormolu just eats it.

Cool. I'll have an internal word at Tweag about that issue & see what needs doing to get it over the line...

Other ormolu notes:

I like that it breaks the import lists only if they're already broken. Short ones (which is most of them) stay on one line.
Having the ability to at least prevent it from running on a file might be useful. It doesn't currently mess up any of our examples (and it appears to not understand our .lhs files, which is probably for the best.), but it would be good to have an escape hatch.

Thanks, I hate it. I mean, I think what I actually hate is explicit import lists, but this really does make the section that I need to scroll past to see anything useful much longer, so I'm not wild about it.

Agree with @michaelpj.

I like that it breaks the import lists only if they're already broken. Short ones (which is most of them) stay on one line.

Yeah, that does sound much better to me than what we see here.

Also I found a bug: https://github.com/tweag/ormolu/issues/463

Okay, I've pushed a new version that:

[...] breaks the import lists only if they're already broken. Short ones (which is most of them) stay on one line.

Invested parties, please take a look. (And bear in mind this is just the stylish-haskell change. To ormolu or not to Ormulo will be a different PR.)

I'm fine with this. And I think this is the same formatting as ormolu would do, so switching to that later won't involve reformatting all the imports again.

If we revisit this it'll need another go.


Advanced command -> "scan" - wrong description

In the table under "Advanced commands" the command "scan" has the same description as "isolate" witch seems to be wrong.

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

ID: 81b2a4c2-e698-dda0-9919-80ca2d05b358
Version Independent ID: 19b5f609-fcaa-cb5b-96b7-462285fd32e7
Content: Investigate entities on devices using live response in Microsoft Defender for Endpoint
Content Source: microsoft-365/security/defender-endpoint/live-response.md
Product: m365-security
Technology: mde
GitHub Login: @mjcaparas
Microsoft Alias: macapara

@msbemba

@ptrsny Thank you for your feedback. The description for Scan is - "Runs an antivirus scan to help identify and remediate malware."
The same has been updated in the document. Thank you for pointing it out.
CC : @yogkumgit This can be closed.
Thanks
Sri

@msbemba Thank you very much for the contribution and sharing this explanation. @ptrsny Hope this comment is helpful for you. If you see a documentation update is required, please feel free to open an issue for the same. We proceed here to close it. Thanks for taking out some time to open the issue. Appreciate and encourage you to do the same in future also.


High tick usage when using DiscordSRV JDBC connection

Description
Luckperms is using tons of tick time to do a function called ExpiringCache.get() when DiscordSRV is connected using a database. You can see it from this spark report (https://spark.lucko.me/zBMwyRdauw). This might be DiscordSRV's issue, but the fact that It is luckperms that's showing up on the spark report is confusing to me, which is why i opened the issue here.
Reproduction steps

Create a Waterfall server, an Airplane server, and a MySQL database with roughly 100ms ping.
Add Luckperms to waterfall server and airplane server, both using the same MySQL database for storage.
Add discordSRV to airplane server and add JDBC string to connect to the database.
Run the servers and add players

Expected behaviour
Server should run normally with no lag from LuckPerms or DiscordSRV. But from running a spark report, it'll show that Luckperms is using most of the server resources on a function called ExpiringCache.get(). This leads to tick spikes of 100ms to 3000ms.
Environment details

Server type/version: Airplane running version 1.16.5 build f5fb024
Server type/version: Waterfall running version 1.16.5 build 412
LuckPerms version: v5.3.30
DiscordSRV Version: 1.22.0

Try disabling the LuckPerms context integration in DiscordSRV, I imagine that is probably what the problem is.

It's a known issue: https://github.com/lucko/LuckPerms/issues/2788#issuecomment-750596964
@Vankka did you manage to get anywhere at the DSRV end? If not, it might be worth automatically disabling the contexts integration if the user is using MySQL...

Ah I should've done some research first, it seems you have, awesome! https://github.com/DiscordSRV/DiscordSRV/commit/e6e4fa3b53d7862a3b6240badef27b3b59ca70d9
@TraceLosu Are you sure you're using an up-to-date version?

The version specified by OP is up-to-date. I've also tried looking through some of the parts that could be the culprit, but haven't found anything. Also not helping matters, the spark report doesn't show any children for ExpiringCache for some reason (I'm guessing it's not taking 2.5s itself)

Also not helping matters, the spark report doesn't show any children for ExpiringCache for some reason (I'm guessing it's not taking 2.5s itself)

ExpiringCache is a cache - and has measures in place (locking) to ensure that only one thread does the actual calculation while the other threads wait.
I think this line is what's causing the problem:
https://github.com/DiscordSRV/DiscordSRV/blob/41b8f04815b028463cbccec42664943b94c199a6/src/main/java/github/scarsz/discordsrv/hooks/permissions/LuckPermsHook.java#L126
It should always use the cached value.

getDiscordId provides the cached value if there is one, and players that are online are kept in the cache (unless mysql takes >30 seconds to respond to the query, which I doubt is happening). I changed it to only use cached data though, it's available on development builds: https://snapshot.discordsrv.com

Great, thank you!

@TraceLosu Could you confirm if fixed please :)

@TraceLosu Could you confirm if fixed please :)

Will do! Just need to wait until my server's scheduled restart.

Yup. The dev build fixed it, no noticable lag.


Copy of Content: Compile list of orgs for usage on Homepage and Our network

User story
As a prospective ReportStream user, I want to know ReportStream's existing senders, so that I feel more confident partnering with RS.
Background & context
The ReportStream team received confirmation from the CDC that we may reference our existing senders by name, but not by logo. This ticket captures the work of compiling the list for use on the Homepage and Our network pages.
Working links

Doc: TBD

Acceptance criteria

[x] Org listed compiled
[x] Org list approved by stakeholders

@chris-kuryak @jillian-hammer follow on from this

@jillian-hammer new content for the home page related to Rhapsody and the sender list is highlighted in the draft doc. Let me know if you have questions!


Add Camera "unexpect failture"

Describe the bug
Add Camera "unexpect failture" on RLC510A. Other (RLC-510WA) works fine
To Reproduce
`Logger: custom_components.reolink_dev.config_flow
Source: custom_components/reolink_dev/base.py:219
Integration: Reolink IP camera (documentation, issues)
First occurred: 21. März 2022, 22:04:44 (2 occurrences)
Last logged: 07:38:57
Unexpected exception
Traceback (most recent call last):
File "/config/custom_components/reolink_dev/config_flow.py", line 70, in async_step_user
self.info = await self.async_validate_input(self.hass, user_input)
File "/config/custom_components/reolink_dev/config_flow.py", line 138, in async_validate_input
if not await self.base.connect_api():
File "/config/custom_components/reolink_dev/base.py", line 219, in connect_api
if not await self._api.get_settings():
File "/usr/local/lib/python3.9/site-packages/reolink/camera_api.py", line 496, in get_settings
response = await self.send(body)
File "/usr/local/lib/python3.9/site-packages/reolink/camera_api.py", line 1721, in send
raise CredentialsInvalidError()
reolink.exceptions.CredentialsInvalidError`
Environment:
Please provide useful information about your environment, like:

Home Assistant version 	core-2022.3.5
Reolink camera model: RLC-510A
Camera firmware number IPC_523128M5MP.764_21121702.RLC-510A.OV05A10.5MP.REOLINK.pak

Hi, I have the same problem, but with the RLC-510WA
`Logger: custom_components.reolink_dev.config_flow
Source: custom_components/reolink_dev/base.py:219
Integration: Reolink IP camera (documentation, issues)
First occurred: 11:08:01 (1 occurrences)
Last logged: 11:08:01
Unexpected exception
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/aiohttp/connector.py", line 986, in _wrap_create_connection
return await self._loop.create_connection(*args, **kwargs)  # type: ignore[return-value]  # noqa
File "/usr/local/lib/python3.9/asyncio/base_events.py", line 1056, in create_connection
raise exceptions[0]
File "/usr/local/lib/python3.9/asyncio/base_events.py", line 1041, in create_connection
sock = await self._connect_sock(
File "/usr/local/lib/python3.9/asyncio/base_events.py", line 955, in _connect_sock
await self.sock_connect(sock, address)
File "/usr/local/lib/python3.9/asyncio/selector_events.py", line 502, in sock_connect
return await fut
File "/usr/local/lib/python3.9/asyncio/selector_events.py", line 537, in _sock_connect_cb
raise OSError(err, f'Connect call failed {address}')
ConnectionRefusedError: [Errno 111] Connect call failed ('<IP_ADDRESS>', 443)
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/config/custom_components/reolink_dev/config_flow.py", line 70, in async_step_user
self.info = await self.async_validate_input(self.hass, user_input)
File "/config/custom_components/reolink_dev/config_flow.py", line 138, in async_validate_input
if not await self.base.connect_api():
File "/config/custom_components/reolink_dev/base.py", line 219, in connect_api
if not await self._api.get_settings():
File "/usr/local/lib/python3.9/site-packages/reolink/camera_api.py", line 496, in get_settings
response = await self.send(body)
File "/usr/local/lib/python3.9/site-packages/reolink/camera_api.py", line 1652, in send
if not await self.login():
File "/usr/local/lib/python3.9/site-packages/reolink/camera_api.py", line 845, in login
response = await self.send(body, param)
File "/usr/local/lib/python3.9/site-packages/reolink/camera_api.py", line 1696, in send
async with session.post(
File "/usr/local/lib/python3.9/site-packages/aiohttp/client.py", line 1138, in aenter
self._resp = await self._coro
File "/usr/local/lib/python3.9/site-packages/aiohttp/client.py", line 535, in _request
conn = await self._connector.connect(
File "/usr/local/lib/python3.9/site-packages/aiohttp/connector.py", line 542, in connect
proto = await self._create_connection(req, traces, timeout)
File "/usr/local/lib/python3.9/site-packages/aiohttp/connector.py", line 907, in _create_connection
_, proto = await self._create_direct_connection(req, traces, timeout)
File "/usr/local/lib/python3.9/site-packages/aiohttp/connector.py", line 1206, in _create_direct_connection
raise last_exc
File "/usr/local/lib/python3.9/site-packages/aiohttp/connector.py", line 1175, in _create_direct_connection
transp, proto = await self._wrap_create_connection(
File "/usr/local/lib/python3.9/site-packages/aiohttp/connector.py", line 992, in _wrap_create_connection
raise client_error(req.connection_key, exc) from exc
aiohttp.client_exceptions.ClientConnectorError: Cannot connect to host <IP_ADDRESS>:443 ssl:default [Connect call failed ('<IP_ADDRESS>', 443)]`
Environment:
Home Assistant version core-2022.3.8
Model RLC-510WA
Build No. build 21121712
Hardware No. IPC_523128M5MP
Config Version v<IP_ADDRESS>
Firmware Version v<IP_ADDRESS>4_21121712
Details IPC_523128M5MPS10E1W31100000001
Reolink IP camera 0.51


SecretManger Secret has a persistent diff, when replicas is set!

Hello!
Issue details
When adding a replica to a SecretManger.Secert in golang:
_, err := secretsmanager.NewSecret(ctx, "my-secret", &secretsmanager.SecretArgs{
	Name:                        pulumi.String("my-secret"),
	Description:                 pulumi.String("My secret"),
	ForceOverwriteReplicaSecret: pulumi.Bool(false),
	Replicas: secretsmanager.SecretReplicaArray{
		&secretsmanager.SecretReplicaArgs{
			Region: pulumi.String("us-east-1"),
		},
	},
})
if err != nil {
	return err
}

I get on all pulumi up call a difference displayed:
Resources:
    ~ 1 to update
    1 unchanged

Do you want to perform this update? details
  pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:dev::aws-classic-test::pulumi:pulumi:Stack::aws-classic-test-dev]
    ~ aws:secretsmanager/secret:Secret: (update)
        [id=arn:aws:secretsmanager:eu-central-1:052848974346:secret:my-secret-css8X3]
        [urn=urn:pulumi:dev::aws-classic-test::aws:secretsmanager/secret:Secret::my-secret]
      ~ replicas: [
          ~ [0]: {
                  ~ region: "us-east-1" => "us-east-1"
                }
        ]

But actually, I can't see any difference in the region field.
Affected area/feature
SecretManger.Secret

Successfully reproduced this issue in TypeScript too:
new aws.secretsmanager.Secret(
  "my-secret",
  {
    name: "my-secret",
    description: "my-secret",
    replicas: [{ region: "us-east-1" }],
  });

Workaround
Applying an ignore to the region of the specific replica will hide this issue:
new aws.secretsmanager.Secret(
  "my-secret",
  {
    name: "my-secret",
    description: "my-secret",
    replicas: [{ region: "us-east-1" }],
  },
  {
    ignoreChanges: ["replicas[0].region"],
  }
);

Investigation Notes
There are additional fields (status, kmsKeyId and lastAccessedDate) which are automatically populated, but these don't appear to be causing the diff. It seems isolated to region field. Tested this by ignoring all fields except region and it still reported the diff.
This could likely be a bug within in the bridge on either the InstanceDiff result that comes back from tf.Diff(), or possibly something which should then get ignored by doIgnoreChanges but perhaps has something missing in the schema to trigger this correctly.

Unfortunately this reproduces still. The root cause here is pulumi/pulumi-terraform-bridge#186

Added to epic https://github.com/pulumi/home/issues/3558


Add accessors for LC header

Introduce get_lc_beacon_slot and get_lc_beacon_root accessors similar to get_current_slot(state) to account for future extensions to the light client header structure that may override how those fields are accessed. Idea is to extend with execution accessors in the future.

Superseded by https://github.com/ethereum/consensus-specs/pull/3190


cip: fix fatal bug in SplitByKnownRegistries

@thockin TL;DR: Allowing images to be pushed to the root directory currently breaks the promoter. Fix on the way...!
Pasted error log from ci-k8sio-cip run from earlier today:
I0304 21:47:40.563384      13 inventory.go:1615] Request {gcr.io/k8s-staging-cluster-api/kubeadm-bootstrap-controller   true}: OK
I0304 21:47:40.566394      13 inventory.go:1615] Request {us.gcr.io/k8s-artifacts-prod/resource_consumer/controller<EMAIL_ADDRESS>false}: OK
I0304 21:47:40.605095      13 inventory.go:1615] Request {gcr.io/google-containers/skydns-ppc64le   true}: OK
I0304 21:47:40.629320      13 inventory.go:1615] Request {us.gcr.io/k8s-artifacts-prod/federation-controller-manager-amd64<EMAIL_ADDRESS>false}: OK
panic: runtime error: index out of range [0] with length 0
goroutine 162 [running]:
sigs.k8s.io/k8s-container-image-promoter/lib/dockerregistry.SplitByKnownRegistries(0xc006ecf590, 0x2f, 0xc000d74000, 0x6c, 0x92, 0x0, 0x0, 0x1, 0xd, 0x0, ...)
	lib/dockerregistry/inventory.go:1491 +0x326
sigs.k8s.io/k8s-container-image-promoter/lib/dockerregistry.(*SyncContext).ReadRegistries.func2(0xc0002ca000, 0xc0000c8240, 0xc00521b4a0, 0xc00113c800, 0xc00113c730)
	lib/dockerregistry/inventory.go:1282 +0xafa
created by sigs.k8s.io/k8s-container-image-promoter/lib/dockerregistry.(*SyncContext).ExecRequests
	lib/dockerregistry/inventory.go:1620 +0x137

(see https://prow.k8s.io/view/gcs/kubernetes-jenkins/logs/ci-k8sio-cip/1235320791898263552)
This happens inside the SplitByKnownRegistries(unparsed_name, prefixes) function, iff the registry name and the full image name path (to be split into the registry + image name) are the same.
I've reproduced this locally, and understand why this is happening. Previously (before the backfill manifests were merged, this has never happened because we've never had to read images that have an unparsed_name that matches a prefix.
Let's take an example. Currently the us.gcr.io/k8s-artifacts-prod/kube-state-metrics repo has some images in it as a result of the backfill. However, there is also a promoter manifest that names us.gcr.io/k8s-artifacts-prod/kube-state-metrics as a destination repository. Normally, new images under this repository would end up as us.gcr.io/k8s-artifacts-prod/kube-state-metrics/<NEW_IMAGE>. And so normally, this would end up calling SplitByKnownRegistries with us.gcr.io/k8s-artifacts-prod/kube-state-metrics/<NEW_IMAGE> as the unparsed_name and us.gcr.io/k8s-artifacts-prod/kube-state-metrics as one of the prefixes to parse with.
However, ever since the partial backfilling that happened yesterday, there are now images that rest in this same path --- as of the time of this writing, for example, this image:
us.gcr.io/k8s-artifacts-prod/kube-state-metrics:v1.4.0

And so when we read the path us.gcr.io/k8s-artifacts-prod/kube-state-metrics today, we end up calling SplitByKnownRegistries with this name that matches one of the prefixes with the same name, us.gcr.io/k8s-artifacts-prod/kube-state-metrics. When https://github.com/kubernetes/k8s.io/pull/623 was initially merged, we did not see the same error because no root-level images had been populated yet. We saw the error today in https://prow.k8s.io/view/gcs/kubernetes-jenkins/logs/ci-k8sio-cip/1235320791898263552 for the first time because some images like us.gcr.io/k8s-artifacts-prod/kube-state-metrics:v1.4.0 were created due to the partial backfill.
The fix should be simple --- if the image name matches a known prefix, just take everything after the last '/' character.
Related context:

https://github.com/kubernetes/k8s.io/pull/623#issuecomment-594908687
https://github.com/kubernetes/k8s.io/pull/629

I noticed that the SplitByKnownRegistries function does not have a corresponding unit test. Fixing that now.


fixsupport date parameter for date_add and date_sub function

Proposed changes
Issue Number: close #xxx

Further comments
If this is a relatively large or complex change, kick off the discussion at<EMAIL_ADDRESS>by explaining why you chose the solution you did and what alternatives you considered, etc...

run buildall

run buildall


Allow to specify ECS container ulimits

It would be nice if there is a way to specify the ECS task limits such as open files without having to provide a complete container definition.
This is needed in some cases where the plan is very big and fails due to the number of providers started/open sockets.

@nikolay-te this functionality is available now with v2.21.0 per #144


SLING-5579: set isActive to true in the activate method

SLING-5579

With the upcoming migration to git this repository will be deleted. If the pull request contains data that should be re-submitted I suggest that you save the changes locally and resubmit them after the migration, which will likely happen on Monday the 9th of October.


Add automatic code coverage as part of CI

Description
This PR adds automatic code coverage reporting as part of CI build using codecov.io
Alternate Designs
N/A
Benefits
See regressions in terms of code coverage
Possible Drawbacks
N/A
Applicable Issues
N/A

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@0a1d530). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #19   +/-   ##
=========================================
  Coverage          ?   98.87%
=========================================
  Files             ?        3
  Lines             ?       89
  Branches          ?        0
=========================================
  Hits              ?       88
  Misses            ?        1
  Partials          ?        0

Continue to review full report at Codecov.

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


SARAALERT-1106: Removes Duplicate Jurisdiction Change in History

Description
Jira Ticket: SARAALERT-1106
This Pull Request addresses an issue where changes to a Monitoree's Jurisdiction would appear twice in the History list. This is due to the fact that we maintain items for Jurisdiction Changes, and a detailed list of all changes. When only the Jurisdiction was changed, there would be two entries (see image).

The fix for this is to filter out jurisdiction_id changes from the Detailed List. This prevents the duplication of data.
This solution has been approved by the PO and Development teams.
(Bugfix) How to Replicate

In master change a monitoree's jurisdiction. Then go view the History. You will see the two entries (like the above image).

(Bugfix) Solution

Now, in this branch, do the same. You will notice that any jurisidiction_id changes are omitted from the Detailed_History History item.

Important Changes
app/models/patient.rb

Removes any changes to the jurisdicton_id from the detailed_history_edit method

Testing
This fix was tested on the following browsers (submitter must check all those that apply):

[ ] Chrome
[x] Firefox
[ ] Safari
[ ] IE11

Why do we still need to create the Jurisdiction history item if this is captured by a Patient update history item? Did the PO team decide that the second text was preferred to the first?

@Bialogs I asked @holmesie which would've been better. He liked keeping the second option. I asked a member of the PO team and they said that they had no strong opinion, as long as the data was not duplicated.

I click on a monitoree -> edit details -> change the jurisdiction I still see two messages

Thanks! It's a weird issue with assignment not working correctly. Using the return-value of reject solves the problem. Good catch Mike!

Can you describe what you mean by "weird issue with assignment not working correctly"? I wasn't actually able to reproduce before you forced pushed. I noticed you also added assigned_user. Please link the corresponding ticket so we can track these changes.

We were using reject! which should update the value that is being .rejected. However, because the value we were using was a method on an object, it was not properly updating the object. Meaning that, while it worked in testing when I used
2.6.6 :007 > x = ["jurisdiction_id"]
 => ["jurisdiction_id"]
2.6.6 :008 > x&.reject! { |apk| %w[jurisdiction_id assigned_user].include? apk }
 => []

it worked. However, when x was replaced with the actual object in the real code, it wasn't properly updating.
The fix that @bialogs and I worked out was to assign the return value of the reject statement to a separate variable, and pass that in.

This needs a rebase


Can't seem to render video link.

Hi,
I am currently using froala in test application.
I render my html using ui-bootstrap popover. But it is not rendering the videos i am attaching using links. Pictures and other stuff works fine though.
Am I missing anything? Or is there any problem?
Thanks.

Found the issue. The angular-sanitize.js and froala-sanitize.js blocks the  tag, You have to place this iframe tag in the list of unblocked items.

@zunairzubair how did you manage to add this? did you modify the code of froala-sanitize.js ?

@noyg yes. Just modify the blockElements variable and add the html tag you want.


Multiple data integration

I have scATAC and scRNA from multiple different samples, can I directly integrate them with glue?

Of course! Just give it a try following the tutorials. If you want to correct for the inter-sample batch effect, you can set use_batch in scglue.models.configure_dataset.
Let me know if there are further issues.


Improve coverage and bump ColorTypes compat

So the 3 simple new tests in test/color.jl are not causing code coverage of a few lines in jim.jl.
Maybe the compiler is optimizing them away and codecov doesn't realize it?
I'll just merge and live with the reduced codecov percentage.


Use REST API to manage courses

As a course manager, I need to manage the course description and details so that the course offering can be included in the course catalog.
Acceptance Criteria:
-I can manage the following information about a course:
--title
--courseType (video, WBT, instructor led)
--description
--instructors
--students
-I can create a course
-I can view a course
-I can modify an existing course
-I can delete a course

Demo video for this is http://www.screencast.com/t/hyoPWI9wyfwR


Replaced alerts with Snackbars for authentication process

I replaced the generic alerts with Material UI's Snackbar for the user authentication process.
Screenshot:

Fixed #6


ArxmlLexer fails on XML comments containing ">"

Hello.
We are facing an InvalidComment issue while loading some 3rd party arxmls containing < / > in comments.
It seems like the lexer does not accept comments as specified by: https://www.w3.org/TR/xml/#sec-comments
I added a test locally to verify:
    #[test]
    fn test_w3c_comment_example() {
        let data = b"<!-- declarations for <head> & <body> -->";
        let mut lexer = ArxmlLexer::new(data, PathBuf::from("(buffer)"));
        assert!(matches!(lexer.next(), Ok((_, ArxmlEvent::Comment(_)))));
    }

It fails.

Thanks for the report!
Including the testcase was very useful, I've committed it with the fix.

Well, that was quick 😄
Thank you very much 👍


Change messaging API

With the current released version (12.0.0) I am not receiving any messages with useNativeMessage. Will this PR fix this?


[$1000]  Composer top padding collapsed in room after scroll up

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

Action Performed:

Go to a room
add some chats until we can scroll up the screen
Scroll to the bottom and scroll upp

Expected Result:
There should be some padding at the top of the composer
Actual Result:
No padding at the top of the composer, it will be notes when hovering over the message which is closer to compose box
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:

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

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.3.37-2
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://github.com/Expensify/App/assets/43996225/f493e7b7-1160-4116-9a4a-d93d066b80cd

Expensify/Expensify Issue URL:
Issue reported by: @wildan-m
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688604509171109
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~016810fbd90ab7373e
Upwork Job ID:<PHONE_NUMBER>491148800
Last Price Increase: 2023-07-21

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

Composer top padding is not visible.

What is the root cause of that problem?

The root cause is that the Participant's Local Time is not visible when in the room thread. But is visible when in Chat thread. Thus showing inconsistency in padding over the composer.

 Chat Thread

 Room Thread

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

Solution 1: Add a top padding to the composer by default.
Solution 2: Render a different component similar to the Participant's Local Time.

What alternative solutions did you explore? (Optional)

N/A

@kavimuru do we need padding there? with padding it will look cropped anyway while user scrolls messages, check my screenshot with added top  padding

I disagree with root cause in submitted proposal because on provided Recording and Screenshot it's chatroom with Concierge and

Participant's Local Time

should not be displayed there, same for #announce chatroom with many Participants, we can't display local time of all Participants

Proposal
Please re-state the problem that we are trying to solve in this issue.
Composer top padding collapsed in room after scroll up.
What is the root cause of that problem?
The root cause of the problem is that there is paddings for scrollable container (chat content). When the user scrolls up the paddings is invisible.

Invisible paddings of chat content

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

So first of all to solve the issue we need to get rid of vertical paddings of chat content. So we need to remove paddingVertical: 16 from chatContentScrollView in styles.js
After that we need to add 16px top padding to ReportActionCompose container. But we need to add padding only in case if we don't need to show Participant Local Time. So for that we need to add  {marginTop: shouldShowReportRecipientLocalTime && hasReportRecipient ? 0 : 16} to the top View in ReportActionCompose component.

What alternative solutions did you explore? (Optional)
N/A

Contributor details
Your Expensify account email<EMAIL_ADDRESS>Upwork Profile Link: https://www.upwork.com/freelancers/~012449b41f0680e59b

Proposal
Please re-state the problem that we are trying to solve in this issue.
Margin top is missed at the top of ReportActionCompose component
What is the root cause of that problem?
If it's multi-user chatroom or concierge room ReportRecipientLocalTime is not appeared and in this case top margin should be add
What changes do you think we should make in order to solve the problem?

https://github.com/Expensify/App/blob/2ec306726201ed7f79d8547a8961c0b41edab271/src/pages/home/report/ReportActionCompose.js#L960
after this line we should add top margin (padding will be displayed incorrect) with
!shouldShowReportRecipientLocalTime && styles.mt4

https://github.com/Expensify/App/blob/2ec306726201ed7f79d8547a8961c0b41edab271/src/pages/home/report/ReportActionsList.js#L159
here to remove bottom padding in scroll area, we should remove this code
shouldShowReportRecipientLocalTime && styles.pt0

and in styles.js replace this code
https://github.com/Expensify/App/blob/2ec306726201ed7f79d8547a8961c0b41edab271/src/styles/styles.js#L1468
with paddingBottom: 16,
What alternative solutions did you explore? (Optional)
N/A

How are these proposals looking @0xmiroslav?

Any thoughts on my proposal https://github.com/Expensify/App/issues/22389#issuecomment-1624580025 ?

paddingTop: shouldShowReportRecipientLocalTime && hasReportRecipient ? 0 : 16

But did you check out the style guide? It says inline styles are not allowed: https://github.com/Expensify/App/blob/main/contributingGuides/STYLING.md#inline-styles

Contributor details
Your Expensify account email<EMAIL_ADDRESS>Upwork Profile Link: https://www.upwork.com/freelancers/~01010c50fbb48ba9b1

@nebiyuelias1 Thanks for the comment! This was the initial decision, it can be refactored a little. So, I've refactored my proposal little bit with adding chatItemWithoutLocalTime to styles.js.

Bump @0xmiroslav.

Proposal
Please re-state the problem that we are trying to solve in this issue.
Composer top padding collapsed in the room after scrolling up
What is the root cause of that problem?
There is no local time for a group, so the <ParticipantLocalTime> is completely hidden.
What changes do you think we should make in order to solve the problem?
We can add a new isContentVisible prop to <ParticipantLocalTime> to hide only the text without hiding the container style.
change this line:
https://github.com/Expensify/App/blob/cc3761bd00c5b8e32883e47c7ac6547f20698369/src/pages/home/report/ReportActionCompose.js#L994
to
                    <ParticipantLocalTime isContentVisible={shouldShowReportRecipientLocalTime && hasReportRecipient} participant={reportRecipient} />

And this line:
https://github.com/Expensify/App/blob/cc3761bd00c5b8e32883e47c7ac6547f20698369/src/pages/home/report/ParticipantLocalTime.js#L50-L62
to
        <View style={[styles.chatItemComposeSecondaryRow]}>
            {props.isContentVisible && <Text
                style={[styles.chatItemComposeSecondaryRowSubText, styles.chatItemComposeSecondaryRowOffset, styles.pre]}
                numberOfLines={1}
            >
                {translate('reportActionCompose.localTime', {
                    user: reportRecipientDisplayName,
                    time: localTime,
                })
            }
            </Text>}
        </View>

If the top padding is too high we can adjust the view height, in my example, we can change the height  to 0 and keep the vertical margin.
        <View style={[styles.chatItemComposeSecondaryRow, !props.isContentVisible ? styles.h0 : {}]}>

What alternative solutions did you explore? (Optional)
N/A

As this is minor style fix, I'd like to pick up the most accurate and simple solution which doesn't cause any minor regressions. No satisfactory proposals yet.

Please, see my proposal. You may have thought that there is some kind of complicated solution, because a lot has been written. But it seems, it will be difficult to find more accurate and simple solution. We only get rid of paddingVertical: 16 in one place and add !isChatItemWithLocalTime && styles.pt4 to other View.
Also I've done small refactoring with extracting new variable:
const isChatItemWithLocalTime = shouldShowReportRecipientLocalTime && hasReportRecipient;
So the simple fix without refactoring will looks in this way, but I think it's better to extract new isChatItemWithLocalTime variable as in my proposal:
chatContentScrollView: {
    flexGrow: 1,
    justifyContent: 'flex-start',
-   paddingVertical: 16,
},

            <View
                style={[
                    shouldShowReportRecipientLocalTime && !lodashGet(this.props.network, 'isOffline') && styles.chatItemComposeWithFirstRow,
                    this.props.isComposerFullSize && styles.chatItemFullComposeRow,
+                  !(shouldShowReportRecipientLocalTime && hasReportRecipient) && styles.pt4,
                ]}

@0xmiroslav Oh, i see. It will be better not removing paddingVertical, but replacing paddingVertical with paddingBottom. So there will be no issues, because of paddingBottom. This variant will be better:

@0xmiroslav thoughts on my main and alternative proposal?

@sarious please update your proposal. Remove all code diffs which are not allowed in proposal stage.

I've updated proposal, removed code diffs.

Bump on this @0xmiroslav.

@0xmiroslav please let me know as soon as you can whether you can review these again. I'll reassign by Thursday otherwise.

I'd like to C+ review this as I already reviewed related issue.

Eugh, automation is great when it works! Alas. Okay @situchan - assigned you as the C+.

@situchan you stepped in here, but we're still waiting on your reviews for the two proposals in here please.

Proposal
Please re-state the problem that we are trying to solve in this issue.
Composer top padding collapsed in the room after scrolling up
What is the root cause of that problem?
There's 16px padding when show local time or hide composer but this is applied to ScrollView's contentContainerStyle so padding part is also scrollable.
https://github.com/Expensify/App/blob/024d210bc0b5a0454b62a72de60057af67cdd045/src/pages/home/report/ReportActionsList.js#L179
What changes do you think we should make in order to solve the problem?
We should move padding outside of scrollable view.

update chatContentScrollView style: paddingVertical: 16 => paddingBottom: 16
remove shouldShowReportRecipientLocalTime || shouldOmitBottomSpace ? styles.pt0 : {} in contentContainerStyle
add shouldShowReportRecipientLocalTime || hideComposer ? {} : styles.pb4 to container View style
To be responsive to archived rooms, anonymous user, public rooms where comment is not allowed, add conditional 16px top margin here: isArchivedRoom || isAnonymousUser ? styles.mt4 : {}

@situchan have you tried the alternative solution?

@wildan-m still not responsive to ArchivedReportFooter, AnonymousReportFooter

@situchan did you have a chance to check my proposal? https://github.com/Expensify/App/issues/22389#issuecomment-1625970966

@mkarkachov Based on https://github.com/Expensify/App/issues/22389#issuecomment-1638602628, I think your proposal was already reviewed.
If I re-review, your solution still doesn't meet https://github.com/Expensify/App/issues/22389#issuecomment-1658039910.

Not sure that proposals before that comment were reviewed but ok

@mkarkachov's proposal looks good to me.
It's the only proposal which covers all possible cases:

composer shown / hidden
archived / not archived
logged in / anonymous

🎀 👀 🎀 C+ reviewed

Sounds good. Thank you guys for the work so far. Let's open the PR 🚀

I will start on PR tomorrow morning and will complete it during a day

Oh sorry @marcaaron, I meant @mkhutornyi
The name was similar and I accidentally tagged wrong name, though I pointed correct proposal link.
I do apologize for everyone.

No worries. I missed that the linked proposal and name did not match up.
Is there any reason why @mkarkachov can't do that proposal? Not sure what else to say other than... please be more careful. I'd rather not take an opportunity away from @mkarkachov because you meant to recommend someone else. 🤷

Is there any reason why @mkarkachov can't do that proposal? Not sure what else to say other than... please be more careful. I'd rather not take an opportunity away from @mkarkachov because you meant to recommend someone else. 🤷

I provided feedback here so @mkarkachov's solution still doesn't cover these cases:

@sarious your solution doesn't work when composer is hidden

It's not difficult to fix my solution. We just need to add paddingTop: 16 to container with ArchivedReportFooter. We can do it by adding styles.pt4 to wrapper View in ReportFooter.js:

So I've updated my proposal, please review it.

I already approved the first completed proposal.
These kinds of things can be easily missed even during PR review. No one has ever thought of archived footer, anonymous footer cases before @mkhutornyi's proposal so it's not true that they could be tackled in PR.

it's not true that they could be tackled in PR.

@situchan We would pay attention in PR to it anyway. I posted my proposal a month ago and #21950  was merged on July 25 to main branch with code
const isArchivedRoom = ReportUtils.isArchivedRoom(props.report);
const hideComposer = ReportUtils.shouldHideComposer(props.report, errors);
const shouldOmitBottomSpace = hideComposer || isArchivedRoom;

It's hard to misunderstand and ignore names of those constants :)

@sarious Uploaded last videos for iOS, Android and Mobile Web - Chrome. PR ready for review #23992

@mkarkachov I tested PR. It really causes this regression.
Another regression:

@marcaaron personally I think no-regression is important factor when accept proposal. No one wants to get penalty because of these minor regressions.
I haven't found any regressions on @mkhutornyi's solution.

@situchan it's normal process for any PR, please pay more attention to your own responsibilities next time to avoid penalty. Can we get fast final decision in this case? I don't want to waste my time, should I apply updates to my PR or will anyone assign this issue to another guy? Who has the last word? @twisterdotcom @marcaaron

@mkarkachov I tested PR. It really causes this regression.
Another regression:

@situchan I made screencast. In my PR there is not regression with hidden compose field, in case if admin restrict posting to another users. Check it please
https://github.com/Expensify/App/assets/1360242/bc3f9173-913d-4e5a-b856-f413ce50c9cc

Discussing internally.

Just in case: contribution guide

Sounds great. @mkhutornyi @situchan we still have time to get a bonus ;) please review my PR. I applied fixes to avoid regression issues mentioned above.

As per https://github.com/Expensify/App/issues/22389#issuecomment-1661061391, @mkhutornyi you can also help testing PR and find any possible regressions.

This isn't overdue. We're waiting for https://github.com/Expensify/App/pull/23992 to hit prod for 7 days.

Payment summary:
@mkarkachov paid $1500 here: https://www.upwork.com/nx/wm/offer/25865850 (Contributor)
@situchan paid $1500 here: https://www.upwork.com/nx/wm/offer/25763008) (Contributor+)
@wildan-m never applied: https://github.com/Expensify/App/issues/22389#issuecomment-1661061745. Due $250 if they do.
@situchan please complete the checklist.

[x] The PR that introduced the bug has been identified. Link to the PR: N/A
[x] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
[x] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
[x] Determine if we should create a regression test for this bug.
[x] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal:
There are many test cases to consider as stated in PR description
So in summary, this can be simplified test step:

Go to any chat room with enough messages history
While scrolling messages, verify that there's always some padding above composer

@twisterdotcom I thought Melvin stored my UW account. Just applied

Ahh yes. @mkhutornyi could you apply for the job as well? Upwork job

@wildan-m you've been paid.
@mkarkachov please accept the offer here: https://www.upwork.com/nx/wm/offer/26194971

@CortneyOfstad I'm out for Parental Leave. Please:

Ask @situchan to complete the checklist.
Pay @mkarkachov $1000 + $500 bonus here: https://www.upwork.com/nx/wm/offer/26194971

@situchan We need you to complete the check list here and link the appropriate details. Thank you!

@situchan We need you to complete the check list here and link the appropriate details. Thank you!

Here: https://github.com/Expensify/App/issues/22389#issuecomment-1680809342

Sorry for the confusion! We ask that you work in the original checklist, otherwise the GH cannot be closed. Thank you!


Mac Catalyst & the start of Multi-Window Support

We are implementing a Catalyst version of our app, and want to support multiple windows as well.
This starts that process by doing three things:

Disable unsafe methods in a Catalyst build
Expose instance functions on SVProgressHUD so you can control N HUDs
Simplify the use of an extension safe version of SVProgressHUD

Thank you very much. Will look into these changes in the next few days. In general, and as a first note, I am missing comments and updates to the documentation (Readme).

@honkmaster sorry about that. I've added some documentation.
I'm not sure what your vision of how SVProgressHUD works in a multi-window world.
A fair amount of the class functions don't make a ton of sense, but it also reduces the simplicity of SVProgressHUD.
Personally, I could see doing something like [SVProgressHUD.sharedInstance show], and deprecating or removing all of the class functions in a future version.
Since we weren't sure what your thoughts would be, @felix8888 only really enabled the methods we use right now. That being said, he's motivated to help refactor the library.


LOTR - 2 Player pong game on FPGA

Screen is 640x580
Display write granularity is 80x120

game tips:

have 3 objects - player1,player2, ball.
player1/2 may get only scalar value for a position on the Y access. (value 0->109) - assuming the length of the player is 10 (corresponding the the Display write granularity is 80x120)
ball gets x & y position. (640x580)
Note: there are 640 pixels on the X access. (but you access 8 at a time -> only 80 x address)
Need a function that can translate the y x position to the Display write granularity + the ability to draw a single pixel within the  write granularity
Deisgn a lookup table to draw all the possible positions of the pixel within the 32 pixel.

Make sure to delete the old ball before drawing a new ball.

No time for this project, will upgrade the snake instead


Added node key CONTROL_STRUCTURE_TYPE to node CONTROL_STRUCTURE.

added the appropriate enum to base.json, this relies on a change in overflowdb-codegen (see linked PR).
adapted the CfgCreator to this change
adapted fuzzyc2cpg to this change

Wont build as https://github.com/ShiftLeftSecurity/overflowdb-codegen/pull/38 is required.

jenkins rebuild


Lazy load css - Bundling fails when used with sass

@duizendnegen @B-Stefan First of all, Thanks to ember-cli-lazy-load, we were able to lazy load our js and making our website more performant.  However over the years our app.css file is hugely bloated and its causing an impact in initial page loading.
We had tried to bundle the app.scss similar to js but we are blocked due to an impediment. We are using sass for styling and the ember-cli-sass takes care of compiling the sass into css. We face an issue where when try to bundle the css, Due to import and extends being used in sass, we ended in duplicating the mixins css code in the bundles.css we generate so that we needed to resolve the dependecies.
Can you help us if u had already tried to lazy load the css along with bundling, and faced an similar impediment like code duplication for mixins using sass. Any help would be much appreciated. Thanks in advance.

Hi @Abuthahir-Sulaiman - I would strongly recommend using https://github.com/ember-engines/ember-engines instead of ember cli lazy load.
We haven't optimized the CSS bundling in this, and aren't planning to maintain this repo anymore.


recoverylog: make playback hand-off fast & deterministic

Formerly, recoverylog playback used behavior of waiting for a timeout
during which no new content became available from the broker. On
reaching that timeout, playback was assumed to be complete and the
consumer would switch from replicating to recording new state changes.
This invited race conditions, where a prior recorder which was delayed
in flushing final writes to the log would race with the new recorder.
Ordinarily FSMHints are sufficient to resolve the forked log history,
but this is a problem for other warm-standby replicas which are
live-tailing the log: depending on which raced write wins, they may
follow a fork of log history which is shutting down, and ignored the new
recorder entirely.
Fix this by changing player behavior to allow for injecting "hand-off"
noop RecordedOps, which serve as write-barriers that effectively give
control of the log to a new recorder in the eyes of any other tailers of
the log. Timeout-based polling is removed entirely.
This has a number of spill-over implications. Playback must now use a
mix of blocking and non-blocking reads, and RetryReader is refactored
to support this behavior. Use of Context is introduced to signal
deadlines and cancellations, and the server-side blocking duration
mechanic is deprecated (in favor of client-side cancellation). The
Author ID must be shared between the Player and Recorder, motivating
a refactor of consumer/master.go.
Also, switch ShardPreInitHook => ShardPostInitHook. In usages such as
ShardIndex, the expectation is the Shard is fully initialized, making a
Post-hook appropriate.

This change is

Publishing partial review feedback.
routines.go has a bug, other items are nits.

Reviewed 17 of 23 files at r1.
Review status: 17 of 23 files reviewed at latest revision, 7 unresolved discussions.

consumer/routines.go, line 117 at r1 (raw file):

func hintsJSONString(hints recoverylog.FSMHints) string {
	if b, err := json.Marshal(hints); err != nil {
		panic(err.Error()) // JSON serialization of FSMHints can never fail.

Nit: I prefer "... must never fail."; with "can", the statement could be interpreted as an claim that it is impossible.
Honestly, this may just be me reading too many RFCs recently.

consumer/routines.go, line 130 at r1 (raw file):

	// Etcd Set is best-effort.
	if err != nil {
		log.WithFields(log.Fields{"path": hintsPath, "err": err}).

hintsPath => key. This compiles because there is a hintsPath func.

consumer/routines.go, line 131 at r1 (raw file):

	if err != nil {
		log.WithFields(log.Fields{"path": hintsPath, "err": err}).
			Warn("failed to store hints")

Change warning message now that this could be a set for other reasons.

consumer/routines.go, line 167 at r1 (raw file):

}

// Deprecated. Stores legacy |offsets| in Etcd.

Nit: Proposed convention for documenting deprecations is to add a paragraph beginning with "Deprecated:".
Example: https://github.com/golang/go/blob/master/src/net/http/transport.go#L107

journal/protocol.go, line 75 at r1 (raw file):

	// not available (eg, because it represents a portion of Journal which has
	// been permantently deleted), the broker will return the next available
	// offset. Callers should therefor always inspect the ReadResult Offset.

Typo: therefor(e)"

journal/protocol.go, line 85 at r1 (raw file):

	// The time at which blocking will expire
	// DEPRECATED. Server-side support for deadlines will be removed. Use
	// context.WithDeadline instead.

See previous comment about godoc convention for deprecation.

recoverylog/recorder.go, line 210 at r1 (raw file):

func (r *Recorder) process(op RecordedOp, b []byte) []byte {
	if r.fsm.NextSeqNo == 0 {

Is this zero-check being removed because a op with SeqNo=0 is expected to be rejected as mis-sequenced?

Comments from Reviewable

Reviewed 1 of 23 files at r1.
Review status: 18 of 23 files reviewed at latest revision, 8 unresolved discussions.

journal/memory_broker.go, line 75 at r1 (raw file):

	}

	for i := 0; true; i++ {

i isn't used elsewhere, could this be for { ... }?

Comments from Reviewable

Finished first pass.

Reviewed 5 of 23 files at r1.
Review status: all files reviewed at latest revision, 15 unresolved discussions.

journal/io.go, line 73 at r1 (raw file):

// NewRetryReader returns a RetryReader at |mark|, and using the provided
// |getter| and |ctx| for all operations.

Nit 1: This is the only new documentation I've seen (so far) that uses the |foo| notation. Is that intentional?
Nit 2: ctx is not an argument. Should it be, or will it be very soon?

recoverylog/playback.go, line 34 at r1 (raw file):

// NewPlayer returns a new Player for recovering the log indicated by |hints|
// into |dir|. An error is returned if |hints| are invalid.

Mismatch between arg name and doc: dir vs localDir.

recoverylog/playback.go, line 61 at r1 (raw file):

// InjectHandoff may be made of a Player instance.
func (p *Player) FinishAtWriteHead() *FSM {
	p.handoffCh <- 0

Is it worth it to panic if more than one invocation is made?

recoverylog/playback.go, line 100 at r1 (raw file):

func (p *Player) Play(client journal.Client) error {
	// Start an adapter to close the playLog Context on cancelCh being selectable.
	// TODO(johnny): Remove after context is plumbed through Play.

Q: Isn't the TODO unnecessary if Player.Play is deprecated? I assume this whole method will be deleted and Player.PlayContext will be called directly.

recoverylog/playback.go, line 214 at r1 (raw file):

	playerStateBackfill playerState = iota
	// Player is tailing new log content as it is written.
	playerStateTailing playerState = iota

Opinion:

playLog treats playerStateTailing identically to playerStateBackfill in all cases. Should they be merged to a playerStateReading?
Rename playerStateBackfill to playerStateBackfilling to be consistent with playerStateTailing.

recoverylog/playback.go, line 216 at r1 (raw file):

	playerStateTailing playerState = iota
	// Player will exit after reading through the current log head.
	playerStateExitingAtHead playerState = iota

Opinion: Rename to playerStateExitAtHead to be consistent in tense with playerStateHandoff*?

recoverylog/playback_test.go, line 512 at r1 (raw file):

func newPlayOperationHelper(c *gc.C) playOperationHelper {
	var err error
	var tpc = playOperationHelper{

Q: What does tpc mean?

Comments from Reviewable

Thanks! PTAL
FYI I've tested using the build branch I'll be putting up for review next.

Review status: 15 of 25 files reviewed at latest revision, 15 unresolved discussions.

consumer/routines.go, line 117 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Nit: I prefer "... must never fail."; with "can", the statement could be interpreted as an claim that it is impossible.
Honestly, this may just be me reading too many RFCs recently.

I actually meant that it is impossible. If you dig into the generated protobuf Marshal (or the stdlib JSON Marshal for that matter), the error return is part of the signature only to allow for embedded user-defined encoding functions which are permitted to return error (and which RecordedOp doesn't use). The generated code never errors, therefor it can never fail.

consumer/routines.go, line 130 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
hintsPath => key. This compiles because there is a hintsPath func.

Done.

consumer/routines.go, line 131 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Change warning message now that this could be a set for other reasons.

Done.

consumer/routines.go, line 167 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Nit: Proposed convention for documenting deprecations is to add a paragraph beginning with "Deprecated:".
Example: https://github.com/golang/go/blob/master/src/net/http/transport.go#L107

Done.

journal/io.go, line 73 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Nit 1: This is the only new documentation I've seen (so far) that uses the |foo| notation. Is that intentional?
Nit 2: ctx is not an argument. Should it be, or will it be very soon?

No, not intentional. I don't have a lot of conviction either way whether we should be using these for godoc comments (perhaps a mild preference to keep it). I do think it adds clarity within function comments.

Added NewRetryReaderContext. I'm not finding the particular golang.org post at the moment, but IIRC the recommended migration pattern is to add FooContext methods, switch call sites to use those, and eventually rename back to Foo when all usages are updated.

journal/memory_broker.go, line 75 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
i isn't used elsewhere, could this be for { ... }?

Huh, would have thought the compiler would catch that. Done.

journal/protocol.go, line 75 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Typo: therefor(e)"

Done.

journal/protocol.go, line 85 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
See previous comment about godoc convention for deprecation.

Done.

recoverylog/playback.go, line 34 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Mismatch between arg name and doc: dir vs localDir.

Done.

recoverylog/playback.go, line 61 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Is it worth it to panic if more than one invocation is made?

added a close, which will cause a future send to panic.

recoverylog/playback.go, line 100 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Q: Isn't the TODO unnecessary if Player.Play is deprecated? I assume this whole method will be deleted and Player.PlayContext will be called directly.

Yes. I just want to clarify again that this is intended as temporary.

recoverylog/playback.go, line 214 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Opinion:

playLog treats playerStateTailing identically to playerStateBackfill in all cases. Should they be merged to a playerStateReading?
Rename playerStateBackfill to playerStateBackfilling to be consistent with playerStateTailing.

the distinction of the two states is used to make sure we close(tailingCh) exactly once, at transition from backfill to tailing, ~L280. Otherwise we'd need another bit somewhere else.

Done, in the opposite way.

recoverylog/playback.go, line 216 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Opinion: Rename to playerStateExitAtHead to be consistent in tense with playerStateHandoff*?

Reworked all of these.

recoverylog/playback_test.go, line 512 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Q: What does tpc mean?

testPlaybackContext, which this type was originally called. switched to poh.

recoverylog/recorder.go, line 210 at r1 (raw file):
Previously, rupertchen (Rupert Chen) wrote…
Is this zero-check being removed because a op with SeqNo=0 is expected to be rejected as mis-sequenced?

This is a hold-over from a very early implementation detail (long since removed) where an FSM with NextSeqNo == 0 was allowed to match any SeqNo.
There's no longer any special handling for zero, and in any case NewFSM initializes NextSeqNo to 1.

Comments from Reviewable

Reviewed 10 of 10 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable


Uses make in windows

This consolidates the build by using the same matrix for Linux, MacOS
and Windows. To do the latter requires installation of CygWin, and to
get GHA to use that requires overriding the shell. One other quirk is
that when you are running tasks via cygwin, it resets the CWD. To avoid
that, this adds the GITHUB_WORKSPACE to .bashrc. Finally, this dodges
various newline related problems and comments why.
Unrelated changes including updating syntax that allows us to
break-apart actions/cache and fixing some lint errors that the build
never caught because lint wasn't run on Windows.

main goal of this is not just coherency, but allowing us to use this to test using make as a part of #316

looks like it didn't like caching cygwin..

last didn't error, but it didn't cache either. I'm trying explicit dir for chocolatey

I'll pull the choco package caching out of scope, as it isn't working, nor is the dominant time in the workflow anyway.

Thanks for the look!

with some adjustments I was able to use the built-in make and save a lot of time and crazy config https://github.com/tetratelabs/func-e/pull/327

https://github.com/tetratelabs/func-e/pull/328 simplified further


Broken with webpack -- Error: Cannot find module "."

As you may know, monaco-editor is broken on webpack projects.  It appears fine at first, but behind the scenes it fails to load webworker modules for language parsing, autocomplete, etc.  There are various workarounds published around the web.  When I found this "react" wrapper for monaco-editor I assumed it would include a fix for it but it does not. As with monaco-editor, react-monaco-editor is broken on webpack. Wrapping monaco-editor in a react component has some value, but the real value would be in having it non-broken with webpack projects, which today is just about everyone.

This module can't do anything about it. Have you tried to use the Monaco webpack plugin from microsoft?

I have same problem. And it is not only on this module, but on the clear monaco too. I've tried to use monaco-webpack plugin, but it caused much more problems like this:
ERROR in ./js/index.js
Module parse failed: Unexpected token (14:4)
You may need an appropriate loader to handle this file type.
| // For more details see: https://facebook.github.io/react/docs/top-level-api.html#react.render
| render(
|     <AppContext.Provider value={{ appName: "My First Frontbench App!" }}>
|         <Main />
|     </AppContext.Provider>
 @ multi (webpack)-dev-server/client?http://localhost:8888 ./js/index.js

I've added Monaco Webpack  plugin to my create-react-app and it solved the problem.

Hi, how do you add webpack plugin to create-react-app without injecting ?

@rizkyramadhan I didn't, I've heard that it's possible but I didn't find how so I ejected 😕

I'm going to close this issue since there isn't much we can do in the react wrapper. Feel free to comment with solutions.


Blockscout smart-contract verification error

Describe the bug
The following error occurs when attempting to verify upgradable proxy smart-contract using command:
npx hardhat verify --network taiko_katla 0x46874eDd92d5A33216fe2e151759Ead76Ff43747

hardhat-verify found one or more errors during the verification process:
Etherscan:
Etherscan API call failed with status 400, response: {"message":"Unknown action","result":null,"status":"0"}

Steps to reproduce

Create a simple upgradable proxy smart-contract
Create a hardhat.config.js file:

const { task } = require("hardhat/config");

require("dotenv").config();

require("@nomicfoundation/hardhat-toolbox");
require("@nomicfoundation/hardhat-verify");
require("@openzeppelin/hardhat-upgrades");

task("deploy", "Deploy contract").setAction(async () => {
  const deploy = require("./scripts/deploy");
  await deploy();
});

/** @type import('hardhat/config').HardhatUserConfig */
module.exports = {
  defaultNetwork: "taiko_katla",
  solidity: {
    version: "0.8.21",
    settings: {
      optimizer: {
        enabled: true,
        runs: 200,
      },
      viaIR: true,
    },
  },
  networks: {
    taiko_katla: {
      chainId: 167008,
      url: "https://rpc.katla.taiko.xyz",
      accounts: [process.env.PRIVATE_KEY],
      gasMultiplier: 1,
    },
  },
  etherscan: {
    apiKey: {
      taiko_katla: process.env.TAIKO_API_KEY,
    },
    customChains: [
      {
        network: "taiko_katla",
        chainId: 167008,
        urls: {
          apiURL: "https://blockscoutapi.katla.taiko.xyz/api",
          browserURL: "https://explorer.katla.taiko.xyz",
        },
      },
    ],
  },
  sourcify: {
    enabled: false
  }
};

P.S. I have tried specifying different options in customChains in apiURL: https://explorer.katla.taiko.xyz/api, https://explorer.katla.taiko.xyz/api (as specified here), but they return other errors:

P.P.S. There is no solution to this problem for hardhat in the documentation either.

Publish the smart-contract with command npx hardhat deploy
Run the verification command npx hardhat verify --network taiko_katla 0x46874eDd92d5A33216fe2e151759Ead76Ff43747

Spam policy

[X] I verify that this issue is NOT SPAM and understand SPAM issues will be closed and reported to GitHub, resulting in ACCOUNT TERMINATION.

@unchase We are aware of several issues with our current explorer, especially regarding contract verification.
But stay tuned, something is coming very soon ;)
For now we will not update blockscout unfortunately


add "pyecharts_jupyter_installer" in the file requirements.txt

It will be better to add "pyecharts_jupyter_installer" in the file requirements.txt

thanks for your suggestion，add already.


[Question/feature] Automatic settings load for neo-cli

@shargon, sometime ago I remember that you implemented an automatic load of plugin settings, right? If the parameter of the config is modified the client automatic reloads it, right?
However, testing https://github.com/neo-project/neo-cli/pull/319 I noticed that it is not valid for neo-cli config.
Do you think that it is worth to do?

i will take a look

With plugins is easier because the classes are smaller, we have this "environment" controlled in one (or few) class. The problem with the global config is that there are a lot of instances using this configuration, we can do it, but i think that is not needed, what do you think?

I think it is not as well. I will close this.


Proposal: Separate identity for remote cluster nodes

We currently have an identity reserved:host which represents both the local node but also all other nodes in the cluster. The collapsing of both local and remote nodes into a single identity is not ideal. With the introduction of the ipcache, we can split the local and remote nodes and assign individual identities.
Use case walkthroughs

Ingress from another node via tunnel, e.g. host networking PID on node A sends traffic to pod on node B.

Traffic encapsulated to other nodes with source identity reserved:host is mapped to reserved:remote-host

Ingress from another node via direct routing

No identity information is carried as PID on node A talks straight to node B

Egress from pod to another node, e.g. pod on Node A sends traffic to node B

ipcache is populated with all node IPs which map to identity of reserved:remote-host
ipcache is populated with local node IP with identity of reserved:host

I assume this proposal supersedes https://github.com/cilium/cilium/issues/3536 .
Case 1 could be achieved on transmit from node A (see that it's from host and rewrite to from remote-node) or on receive at node B (there's no legitimate reason with this model to receive from:host so from:host can be rewritten to from reserved:remote-host).
Case 2 depending on how it's received we could treat as from:world by default but if the source IP is known and can be matched against another node, it could be considered from reserved:remote-host.
There's just one tricky part here, which is how we handle upgrade. If anyone has written any policy depending on the current classification behaviour for other nodes in the cluster, then we change the way the traffic is classified, then it will break their policies. We may need to consider another thing similar to
the host-allows-world option. For case 1, it's more likely to break because the traffic is currently considered as host, which is more restrictive. If someone allowed traffic from:host and the traffic is now considered from:remote-host then the old policy would no longer allow traffic based on the new behaviour. Case 3 should be unaffected as currently the destination would map to cluster / world, and policies that select reserved:world should probably apply to remote-host as well - ie, if you are allowed to talk to the world, then you are also allowed to talk to remote-host.

I assume this proposal supersedes #3536 .

Yes, I was looking for this but didn't find it.

We may need to consider another thing similar to
the host-allows-world option.

Agreed. There is no way around it. It's invisible for new users so I'm not worried.


Update fees on for bots

Gas Refund of 0.015ETH / 0.0015BNB/ 0.15Matic
Transaction fee of 0.5% (original token amount)
Requirements:

Gas Refund fees go into the gas-refund wallet
Transaction fees go into a separate Paloma EVM wallet. Please confirm whether this add more costs?

Requires that all bots are stopped and the Vyper contract replaced.

@webelf101 is this complete?

Please use this address for fees: 0x7a16fF8270133F063aAb6C9977183D9e72835428

Closing as fixed.


Invalid geometry error with generate_matching_area.py despite validity checks

I got an invalid geometry error at the generate matching area stage with a shapefile (available at /maps/aew85/projects/calidris.geojson). I loaded the shapefile into QGIS, ran a geometry check but it didn't seem to contain any invalid geometries. I also ran an ogr validity check and did a -makevalid to be safe. The updated shapefile is available at /maps/aew85/projects/calidris_valid.geojson. However, I got the same error again. Wondering if this is to do with the highly fragmented nature of the shapefile (see screenshot) - it's a bit of a weird one.
The command I ran is as follows:
tmfpython3 -m methods.inputs.generate_matching_area --project /maps/aew85/projects/calidris_valid.geojson \ --countrycodes /maps/aew85/tmf_pipe_out/calidris/country-list.json \ --countries /maps/4C/osm_boundaries.geojson \ --ecoregions /maps/4C/ecoregions/ecoregions.geojson \ --projects /maps/mwd24/tmf-data/projects \ --output /maps/aew85/tmf_pipe_out/calidris/matching-area.geojson
NB I was on my branch aew85-ex-ante-offset. But I haven't changed that script so it shouldn't make a difference.
Error is as follows (sorry I don't have the full version):
shapely.errors.GEOSException: TopologyException: side location conflict at -76.453624184629916 4.0141383638295451. This can occur if the input geometry is invalid.

Solved by manually resolving all self-intersections in QGIS (thanks Tom for the suggestion!). I hadn't noticed these initially because they weren't showing up on any of the validity checks I ran either in QGIS or the command line - not sure why. Attached screenshot shows example of self-intersection in original shapefile (green) and cleaned version (grey). The clean shapefile is now available at `/maps/aew85/projects/calidris.geojson'.


 These allow
redistribution with the semantics of the existing STRICT and OFF
message-load-balancing types respectively. Previously you could only get
redistribution if you were using ON_DEMAND. The semantics for STRICT and
OFF were not changed so as not to impact existing users.

@AntonRoskvist, can you give this branch a test?

Sure can, @jbertram .
Initial tests are looking great! I will keep this running in a "busy" test environment over the weekend to weed out any potential issues, but for the past few hours it's looking good. The system is mostly behaving as expected, with the load balancing type "REDISTRIBUTION_ONLY" configured. Forwards are WAY down and the system is running much smoother.
The only odd behavior I noticed  was in regards to queues that require redistribution. As an example: If a consumer is connected to broker "1" on a new queue and then messages get produced to broker "2", the messages will not get forwarded but rather backs up on broker "2". If I disconnect and reconnect the consumer on broker "1", then redistribution starts and keeps on going as expected.
If messages are produced on broker "2" first and a consumer later connects to broker "1", then all works as expected. Otherwise it's looking fantastic!
Br,
Anton

@AntonRoskvist so the "odd" behavior your seeing isn't specifically related to this PR. It is simply the way the broker deals with auto-created queues in a cluster at the moment. I'll try to explain...
Each broker has its own set of addresses and queues - even in a cluster. When a queue is created on one broker in a cluster the broker on which the queue is created creates what we call a "local" binding. It then informs all the other nodes in the cluster about this binding at which point the other nodes create a "remote" binding. Therefore, at this point all the nodes in the cluster know about the queue. Likewise, when a consumer is created on a queue on one broker all the other nodes are informed about it. If another broker in the queue has a matching local binding with the same name (i.e. the queue has actually been created on that broker) then a "redistributor" will be created on that queue so that messages sent to it can be redistributed if necessary. In your use-case there is no local binding so the redistributor isn't created.
As I mentioned previously, this issue wasn't caused by this PR. IMO a new JIRA should be opened and a new PR should be sent to address it. In the mean-time you can work-around the issue by pre-creating all the queues for which you need redistribution.

Oh, okay. In that case I think this PR is looking good. I have been running it through heavy load tests continuously since Friday and I have seen no issues except the one I mentioned above.
I'd be happy to publish a new Jira for the other minor problem, but I am not quite sure how to formulate it... This is the expected behavior right now and we might want to alter how or when redistributors are formed, is that right?
Unfortunately I can not configure static queues as I am working with several hundreds of them, in several sites that are created ad-hoc by external teams/components that I have little to no control over. But I'll work out some temporary solution for that, like forcing disconnects for consumers on the queues that are not currently getting redistributions.
Br,
Anton

My vote is to just respect redistribution-delay with STRICT.
When we wrote STRICT, we thought we didn't need redistribution, as with forward-when-no-consumers=off, it would mean the queue would only receive messages if there was a consumer.
so, if in fact redistribution-delay and STRICT makes sense, just change STRICT. There's no sense on adding a third option.
if you have a strong opinion about it.. call it something else.. I suggest... ROUND_ROBIN, make it the default, and deprecate STRICT. I see no value on STRICT after the change though.

ROUND_ROBIN is actually not a bad name if you want to keep a third option.

ha! I just googled.. and most other-brokers I just found actually call that kind of distribution... guess what? ... "ROUND ROBIN"

well.. it's OFF_WITH_REDISTRIBUTION. I got it wrong.
But still.

All things being equal I would agree with you, @clebertsuconic. However, as noted in the commit message, I decided not to change the semantics of STRICT and OFF as I didn't want anyone using those settings along with a non-zero redistribution-delay to suddenly start having redistribution when up until this commit the redistribution-delay would just be ignored in those cases. Certainly an argument could be made that such a configuration is rare and might even be considered "wrong." However, the fact remains that using STRICT or OFF with a non-zero redistribution-delay has always been a technically valid configuration and the documentation explicitly states that one must use ON_DEMAND to enable redistribution. I just think changing semantics in a minor version like you're suggesting is an objectively bad thing.
Perhaps this should just wait until the next major release where we can properly break stuff. What do you think?

Hi,
I don't really know if this is expected behavior or not, but I have run this PR in one environment since it was created and just noticed something. When running with REDISTRIBUTION_ONLY, topics (multicast) are not forwarded across the cluster, so given a cluster of broker A and B, messages produced to broker A only get delivered to subscribers directly connected to broker A. Subscribers connected to broker B gets none. If no consumers are connected to broker A then the message is lost (using non-durable topics). Using ON_DEMAND the redistribution seem to work as expected, but then all anycast queues get distributed as well, which is what I was hoping to avoid in the first place.
I guess this might relate to the other issue I had where remote bindings don't get created unless there is also a local one present?
Besides this I have had no issue running this PR, even ported into the latest version of the broker
Br,
Anton

@jbertram I have made a fix to the issue I mentioned above and also fixed the conflicts in the tests. How would I go about sending in that fix? Can I just append it here somehow?

@AntonRoskvist, push your branch to GitHub somewhere and let me know where it is. Then I can cherry-pick your commit onto mine and update the PR.

Okay cool, you can find it here:
https://github.com/AntonRoskvist/activemq-artemis/tree/ARTEMIS-1925-CHANGES
Actual fix is on line #482 in BindingsImpl.java
I built this against tag 2.16.0 as master branch is currently not building
Br,
Anton

@AntonRoskvist, I couldn't find any change on line 482 of the BindingsImpl on commit d4c502d of your branch. Also, it looks like you squashed your changes and my changes together which makes it hard to identify what you actually did change. In any event, I've resolved the conflicts and pushed again. Let me know how I can get your fix onto my branch.

@jbertram
Hmm, I don't really know how to share that in some simple way then...
The reason you are not seeing any change is probably because compared to the master branch there is no change, it's a revert of that code line from what is in your original pr back to what master branch is using. So now it looks like it's on line :489, I changed:
if ((messageLoadBalancingType.equals(MessageLoadBalancingType.OFF) || messageLoadBalancingType.equals(MessageLoadBalancingType.REDISTRIBUTION_ONLY)) && theBinding instanceof RemoteQueueBinding) {
back to:
if (messageLoadBalancingType.equals(MessageLoadBalancingType.OFF) && theBinding instanceof RemoteQueueBinding) {
It's in the fork I shared, but maybe the best would be if I worked from your new pr and made just this change to that?
Apologies for possibly making this more complicated than it needs to be, I'm not really used to this workflow.
Br,
Anton

@AntonRoskvist all you need to do is take my branch, add your changes on top of it, and then push it to your repo. Then I can take your changes and cherry-pick them onto my branch here.

@jbertram
Okay, then you can find it here:
https://github.com/jbertram/activemq-artemis/pull/12


Usage of library is missing

Could you please provide example how to use library?

Sorry, I've been out of band for some time, and engaged in other projects.
I have this in my backlog of personal issues, and I'll come back at it.
This started as a personal project, but it makes a lot of sense to put some additonal effort in using it properly. In short, it is a tool for adding more metadata to swagger, such as varying response metadata with models and examples.
You can see an example of usage in the tests folder, like the following:
@responds(500, "On server failure",
          schema={
              ('details', 'norequired'): 'string',
              'required-details': {
                  ('key', 'required'): []
              }
          })
@responds(200, "Always", schema={})
class ExampleView(APIView):
    "Example big comment"
    permission_classes = [permissions.IsAuthenticatedOrReadOnly]

    def get(self, request, *args, **kwargs):
        "Example get comment"
        return Response()

    def post(self, request, *args, **kwargs):
        "Example post comment"
return Response()

That is no excuse for proper documentation and improving the README in general, but if you find this interesting, please let me know!


Separate Hardware and Platform Manufacturers to allow Platforms by different vendors on hardware not produced by that vendor

Environment

Python version:  3.x (migrated to v3 yesterday so should be latest version)
NetBox version:  2.4.3

Proposed Functionality
Allow an additional Manufacturer to be selected for Platforms within a device.  This would avoid the issue above and still limit the list of available Platforms to the select Manufacturer as originally requested in #1744
Use Case
Dell R630 device with Dell selected as the Manufacturer with VMWare ESXi Platform installed.
Database Changes
Possibly, as this would need a new field to be present on the Device form with the appropriate logic to filter our Platforms for that Manufacturer.  but this may not need much, if any Db changes to achieve it.
External Dependencies
None that I know of.

I think the goal of allowing a platform to be associated with a manufacturer is specifically to enforce this limitation. In this case it would make sense to not treat VMware as a manufacturer, and simply refer to the platform as "VMware ESXi."

  I.e HP and Dell amongst other.
The same issue arises for windows and all its variants running on physical hardware.
I suppose I could remove the associated manufacturer from the platform but then I’d end up with a long list of platforms for all devices, which essentially undoes what the original request to restrict that list was intending.
I don’t have to do this for VMs, so I’d expect the same hardware devices.
Chris Jenner

On 16 Aug 2018, at 16:34, Jeremy Stretch<EMAIL_ADDRESS>wrote:
I think the goal of allowing a platform to be associated with a manufacturer is specifically to enforce this limitation. In this case it would make sense to not treat VMware as a manufacturer, and simply refer to the platform as "VMware ESXi."
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

 I.e HP and Dell amongst other.

Yes, because ESXi is not inherently bound to any particular manufacturer. This is contrast to a platform like Junos, which (typically) runs only on hardware manufactured by Juniper.

I would disagree there.  Whilst some platforms are created by hardware vendors, a lot are not.  In these cases they may be called something like "Software Vendors" or "Developers".  I would suggest "Vendor" covers both hardware and software creators for all platform types.  I can see why you have a Manufacturer type in NetBox, allowing for Device templates to be associated with a Hardware Manufacturer.  But, in my experience, most Data Centers will predominantly have more devices with platforms not created by the hardware manufacturer than not.  (i.e. Hypervisors, Windows, Linux, Unix Platforms running on a variety of different hardware types).
Currently creating Platforms with no associated Hardware Manufacturer undoes the change requested in #1744 by listing all platforms unrelated to the hardware selected.
To more accurately document platforms I believe removing their link to the hardware manufacturer field and instead creating a new field (called "platform vendor" for example) so that you can accurately document vendors for platforms fulfills the requested function change in #1477 and allows for completely documenting all environment types.
Not being able to set a Vendor for soo many platforms sets my OCD twitching.  It also seems to be at odds with creating a system to accurately document Data Center Environments.

Just chiming in that I have the exact same issues and requests as other people above. We have a lot of switches from different manufacturers running the same NOS distribution. While it would be possible to just decouple the platform from the manufacturer, that feels like a subpar solution, especially for instances where there are multiple platforms from a single software vendor in use.
What would be nice is if we could have a new field for manufacturers that tells if they are vendors for software, hardware or both. This would decouple the hardware manufacturers from the platform, which are very often unrelated to each other. As it is, creating multiple identical platforms just so that we can add those platforms to devices of differing manufacturers is a hacky solution.
Ideally we could then store version information on platforms and tie in those platforms with the services (service templates?) on individual devices, but I feel that that should be a separate issue.

Yes this is really causing me a headache.
I have several Hyper-V clusters and the "devices" that make up the nodes of the clusters are "HP" and I can't select "Server 2016" as a platform because it's a Microsoft product. This makes no sense for servers, only for networking equipment.

Same here, I have physical servers from HP (Manufacturer=HP)  with Windows installed (Platform=Microsoft), but I replicate this on Netbox, really a pity :(

+1 Same Issue here.
Have multiple Servers from Dell, HP and some Whiteboxes running 2 V-Sphere cluster.
Would be nice to have the ability to put Dell as the Manu for the server, VMWare as the manu/dev/whatever of the Operating System.
unfortunately Jeremy never hat this case and/or rageclosed the thread after recieving 2 downvotes for his descision in :

So I would have to create VMWare ESXi as a platform for all Manufacturers on which you run that platform? I.e HP and Dell amongst other.

Yes, because ESXi is not inherently bound to any particular manufacturer. This is contrast to a platform like Junos, which (typically) runs only on hardware manufactured by Juniper.

so  @jeremystretch maybe u like to reconsider to let the user decide (maybe via a setting) if he want to be dell or vmware the manufacturer of the operating system?