Director::absoluteURL is weird

Affected Version
4, 5
Description
This is a follow up on https://github.com/silverstripe/silverstripe-framework/pull/9074/files#r298450879
Director::absoluteURL method does not seem to have any means to produce proper absolute URLs including SS_BASE_URL. As long as input url is non-relative, it will always ignore SS_BASE_URL.
This behaviour also seems to be covered by the tests.
Basically these tests cover the behaviour of ignoring the base url:
Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/mysite/');
$this->assertEquals("http://www.mysite.com:9090/root", Director::absoluteURL("/root", Director::BASE));
$this->assertEquals("http://www.mysite.com:9090/root", Director::absoluteURL("/root", Director::ROOT));
$this->assertEquals("http://www.mysite.com:9090/root", Director::absoluteURL("/root", Director::REQUEST));

That means we need to either "sanitize" the input URL before passing it to that method, or don't use the method.
Another option is to deprecate it and provide more clear APIs around working with URLs?

Just clarifying my understanding of the problem.
Let's say your site is accessible under 2 domains: example.com and example.org. You want example.org to be the primary domain, so you set that as your SS_BASE_URL.
However, when a user access the site on the DotCOM domain and the site outputs an absolute URL, it ignores the SS_BASE_URL value in favour of the domain used to access the site.
Is that the behaviour we are finding problematic?

It’s absoluteURL(), not canonicalIRL(). 😉

My thinking aligns with @unclecheese. The primary use case for absoluteURL is to create a URL that will work in Emails or in other places outside of SilverStripe.
Although there could be a use case for canonicalURL as well.

I reckon if nobody knows what is this method for and why it's doing what it's doing, we should deprecate it and provide several others, which would be more SOLID :)

I reckon if nobody knows what is this method for and why it's doing what it's doing, we should deprecate it and provide several others, which would be more SOLID :)

It's ensuring that any url you pass into it is prefixed with the appropriate protocol, hostname, and base url.
As an aside, make sure you are only using alternate_base_url for testing purposes (i.e. to override SS_BASE_URL), since it interferes with the operation of urls on live sites. That may be what you're experiencing?

@dnsl48 I answered your concern on that other PR at https://github.com/silverstripe/silverstripe-framework/pull/9074/files#r317874705

It's ensuring that any url you pass into it is prefixed with the appropriate protocol, hostname, and base url.

Yes, that's been my assumption as well. However, as I mentioned above, it seems to be producing incorrect results for absolute URLs.

As an aside, make sure you are only using alternate_base_url for testing purposes. That may be what you're experiencing?

Good to know, cheers :)
However, that's not the case. The snippet I put in the first message is a copy-paste from the framework tests covering the discussion subjectp

it seems to be producing incorrect results for absolute URLs

Maybe that's correct result from some perspective, but at least 3 developers have already been confused by what the method actually does. Seems likely the method has misleading name/behaviour/documentation.

This looks like a bug to me, if your base URL includes a sub-folder then absoluteURL() should respect that (IMO)

However, when a user access the site on the DotCOM domain and the site outputs an absolute URL, it ignores the SS_BASE_URL value in favour of the domain used to access the site.

This behaviour is intentional in order to support modules like subsites, by the way. Not that this is necessarily correct (one could argue the SS_BASE_URL should supersede this), but we would need to be careful in changing this behaviour.
Personally, I dislike the Director::REQUEST behaviour of Director::absoluteLink(), and it was maintained primarily to support old legacy behaviour (see the deprecation in the top of the method). I'd be happy to drop this entirely in ss5 to help make the logic more clear. :)
In fact, instead of passing the base "type" as the second argument, we could probably just allow the users to pass in the base path explicitly; If the user wants a "current request" based url, they could pass it in as second, otherwise it would default to the SS_BASE_URL.
I suggest to keep the is_root_relative() check, in order to prevent the double base url combination, however,
How about the main body look something like this.
    /**
     * Turns the given URL into an absolute URL. By default non-site root relative urls will be
     * evaluated relative to the current base_url.
     *
     * @param string $url  URL To transform to absolute.
     * @param string $base Base URL to use, otherwise SS_BASE_URL will be used
     *
     * @return string
     */
    public static function absoluteURL($url, $base = null)
    {
        // Use base provided, or default to SS_BASE_URL
        $parent = $base ?: Director::absoluteBaseURL();

        // Absolute urls without protocol are added
        // E.g. //google.com -> http://google.com
        if (strpos($url, '//') === 0) {
            return self::parseProtocol($base) . substr($url, 2);
        }

        // Already absolute
        if (self::is_absolute_url($url)) {
            return $url;
        }

        // If url is root-relative, strip subdirectory off base to prevent double-added subdir
        if (self::is_root_relative_url($url)) {
            $parent = self::parseProtocol($base) . self::parsehost($base);
        }

        // Map empty urls to relative slash and join to base
        if (empty($url) || $url === '.' || $url === './') {
            $url = '/';
        }

        return Controller::join_links($parent, $url);
    }

@dnsl48 thoughts?

If you want to fit this into ss4, you could make a new method, and deprecate the old one, however. :) @robbieaverill ?

I guess with the above code, you could always make Director::baseUrl() injectable by subsites?

I suggest to keep the is_root_relative() check, in order to prevent the double base url combination, however.

I cannot think about use cases where it's required. On the other hand, I already had a couple use cases where is_root_relative() prevents that method from being useful.
I think we could come up with a list of use cases we want to address by this particular method, then would be more clear what it should and shouldn't do.
My feeling we might potentially even split SS_BASE_URL into two different vars: SS_BASE_ROOT and SS_BASE_HOST and then check for substr(...) !== $SS_BASE_ROOT instead of is_root_relative, as long as the method needs that concern.
Another potential thing to consider is splitting concerns between two methods: absoluteURL and canonicalURL (I guess that might make sense in context of subsites module?).

I cannot think about use cases where it's required. On the other hand, I already had a couple use cases where is_root_relative() prevents that method from being useful.

Your SS_BASE_URL is http://localhost/mysite/. You have a link you parsed from a HTML text block with  and you want to convert it to an absolute link.
In your PHP code you run Director::absoluteLink('/mysite/about-us').

With the check, you'll correctly get http://localhost/mysite/about-us.
Without the check, you'll incorrectly get http://localhost/mysite/mysite/about-us.

In the above situation, you have another link .
If you click that link in your browser, you will navigate to http://localhost/mysite/about-us. The PHP code needs to replicate the same logic as the browser, otherwise you'll end up with inconsistent behaviour when resolving links on the serverside.
Let's say you have another link in the same situation above.  You want to get the absolute URL of this, so you run Director::absoluteLink('contact-us'). Because this is a non-root relative url, our web browser would have resolved this link differently, appending it to the base tag instead, sending us to http://localhost/mysite/contact-us. Thus, our PHP code replicates the same logic, by appending it to the base instead of stripping the trailing path component (/mysite/).
Please check https://mor10.com/html-basics-hyperlink-syntax-absolute-relative-and-root-relative/ and make sure you understand the distinction.
The important silverstripe-specific principle here is that, when using  tag (which SilverStripe currently relies on), root relative URLS IGNORE the path component of the base tag, where non-root relative urls append to the base.

I think we may need to define some common terminology. I suggest we refer to the following RFCs

RFC 1738: URL, Syntax
RFC 1808: Relative URLs, BNF

Simplifying the BNFs from RFC1808 we have the following:

Absolute URL == http://localhost/root/page/
Relative URL == //localhost/root/page/ || /root/page/ || root/page
Absolute Path == /root/page/
Relative Path == root/page

Here's the use cases I see and derived from the discussion so far:

Build an abs url for rel paths
Build an abs url for abs paths excluding web root subfolder
Build an abs url for abs paths including web root subfolder

My guess is we're thinking in different contexts about how absolute paths should be handled.

In your PHP code you run Director::absoluteLink('/mysite/about-us').

I'm guessing you meant Director::absoluteURL by that and not absoluteLink.

You have a link you parsed from a HTML text block with  and you want to convert it to an absolute link.

Feels like an edge case to me as long as we're talking about backend application?
My main use case for this method so far was to convert routes (read url paths from within yaml configuration into URLs to use them for redirections. In this context considering SS_BASE_URL as a base tag for an html document doesn't make much sense. It should rather be considered as an application subfolder of the web root which should be added for absolute paths as well as for relative ones. Use relative paths for configuring routes doesn't feel semantically correct since that should indicate a route relative to a document and not to the application base.

I'm guessing you meant Director::absoluteURL by that and not absoluteLink.

Yes sorry.

Feels like an edge case to me as long as we're talking about backend application?

It's less of an edge case than you'd think; E.g. HTML editor field generating content for an email could use root relative links (or even simply, static templates with such links passing through an absolute link rewriter).

My main use case for this method so far was to convert routes (read url paths) from yaml

There are a few options you could use in this case:

You should just use Controller::join_links() if you want to ignore url resolution rules, and you want to "force" it to act as a base-relative path.
You could ltrim /
Just make sure your url paths don't have leading slashes. (i.e. follow the convention we use in Director.rules config) if they base- and not root- relative.

There are a few options you could use in this case

Yes, I agree, these are valid options for working around.
On the other hand, our APIs around working with URLs is still confusing for developers, which this issue brings up.
Also, looks like Director::is_absolute_url does treat a subset of relative urls as absolute, which is semantically incorrect (accordingly to the RFCs I provided above).

Nearly every Link() method produces a root relative url, but the way.
What relative urls are treated as absolute?

//localhost is not a relative link. It's a protocol-less absolute link. I'm happy to be proved wrong if that's the case though.

What relative urls are treated as absolute?

That's correct and documented feature though :) Still, semantically incorrect. This one: //localhost/path is a relative URL, accordingly to the RFC 1808

Ok, according to the rfc it should have a protocol to be absolute. I'm happy for you to suggest a change to remedy that in the is_absolute_url  method.

I'll get back to you when I'm back in the office tomorrow. In the mean time please look at the examples I have above and let me know how you feel we should change this api.

Nearly every Link() method produces a root relative url, but the way.

Sorry, I'm not familiar with that terminology too much, so I've been trying to find the common ground by referring to the RFCs above.
To clarify what I mean by that, here are a couple of ideas accordingly to RFCs:

There are 2 different entities: url and path
Either of them can be absolute or relative
Absolute URL always has a scheme
Absolute Path always has a leading slash
Relative Paths never have leading slashes
Relative URL may either start with //host/ or be a path (either absolute or relative)

My feeling is that the use case convert an absolute path to an absolute url is hard to achieve by existing APIs.
Another question is where the absolute path comes from and what it indicates.

how you feel we should change this api

Before suggesting the particular changes I'd like to put together a list of use cases we want it to cover.
I'll start doing that in this post.

Building URLs for HTTP redirects

HTTP 1.1 Location header supports both Absolute URL and Relative URL. RFC 7231

HTTP 1.0 Location header only supports Absolute URL. RFC 1945

As such, for keeping backward compatibility with HTTP 1.0, as the best practice we may want to always generate absolute URLs for redirects.
As such, I can see 2 scenarios:

generate Absolute URL for Absolute Paths
generate Absolute URL for Relative Paths

In case we get the redirect address from YAML configs, we may want to:

add SS_BASE_URL to it if it's an absolute path
add REQUEST_URI path to it in case it's a relative path

HTTP 1.1 is 20 years old; Do you think we need to worry about 1.0 support? I agree we should never provide relative urls, but at the very least, root relative urls should be safe enough, and are the current convention in silverstripe core.

add SS_BASE_URL to it if it's an absolute path

I've explained in detail why you can't add the whole base url to absolute paths; You can only add the protocol and hostname to resolve these urls to get a correct result. Please read above again, especially the example I gave with $page->Link().

add REQUEST_URI path to it in case it's a relative path

$Link in templates would never follow this resolution method, because it would always use the <base > tag (SS_BASE_URL) as I have also explained. If you use the whole REQUEST_URI you will end up with inconsistent behaviour. E.g. a link from http://localhost/about-us to about-us/our-staff would end up as http://localhost/about-us/about-us/our-staff.

@robbieaverill @chillu could either of you please check our assumptions here to make sure we come to a resolution. It may be blocking #9207

HTTP 1.1 is 20 years old; Do you think we need to worry about 1.0 support?

Well, that's a controversial question. However, here's a long living stackoverflow topic which has some stats from someone's random internet servers: https://stackoverflow.com/questions/2073392/is-http-1-0-still-in-use
I wouldn't confidently say 1.0 is not in use anymore.

I've explained in detail why you can't add the whole base url to absolute paths ... Please read above again, especially the example I gave with $page->Link().

It feels like you're mixing up hyperlink with url path and url. Sorry, I'm not exactly sure how Page->Link() is related to what absoluteURL should be doing? These are not really interchangeable things.
Hyperlink is a part of HTML specifications, whereas URL and url-path is from HTTP specifications (I've given a number of RFCs about that above).
If you're saying: we cannot do that because we have Page->link implemented in this way, then it feels similar to what this issue is about - we have APIs which are confusing for developers.

$Link in templates would never follow this resolution method,

This is exactly why I'd like to write up the list of use cases before we discuss particular API implementations.
You're talking about generating hyperlinks, I'm talking about generating HTTP redirects - those are different use cases and may need different approaches.

Sorry, I'm not exactly sure how Page->Link() is related to what absoluteURL should be doing?

SiteTree::AbsoluteLink() calls this method to generate it's absolute link. return Director::absoluteURL($this->Link($action));
SilverStripe must be internally consistent. This means that both template logic and redirects must be consistent with one another; This is why Link() generates root-relative links, in order to ensure resolution is consistent.
SilverStripe wasn't developed with RFC consistent terminology in mind, so try not to get too hung up on terms used internally.

E.g. a link from http://localhost/about-us/ to about-us/our-staff would end up as http://localhost/about-us/about-us/our-staff.

That is expected behaviour IMHO.

Only without a base tag

Clarifying the docs is a good start.
Although, the documented behaviour itself is confusing for a lot of devs (at least 5 people I've counted so far including myself).
If you think that's not a problem I'll close this issue in a week. Otherwise, I'll keep writing down the use cases which we want to cover by the APIs and then I'll come up with some suggestions for refactoring it.
Please upvote this post if you think it's worth refactoring


Accessing children prop outside of JSX triggers hydration warning

Describe the bug
Per title - receive hydration warnings when accessing the children prop in any way (besides rendering props.children of course) in SSR context.
export const Stack: ParentComponent = (props) => {
  // Accessing children in any way (even just a console.log) causes hydration warning
  console.log(props.children);

  return props.children;
};

For context, I came across this while trying to create a "stack" component that injects a divider element between children (simplified below):
export const Stack: ParentComponent = (props) => {
  return (
    <For each={Array.isArray(props.children) ? props.children : [props.children]}>
      {(c, i) => {
        return (
          <>
            {/* add a divider */}
            {i > 0 ?
 : null}

            {c}
          </>
        )
      }}
    </For>
  );
};

Your Example Website or App
https://stackblitz.com/github/marbemac/solid-streaming-lazy-bug/tree/children-bug
Steps to Reproduce the Bug or Issue

Go to https://stackblitz.com/github/marbemac/solid-streaming-lazy-bug/tree/children-bug
Click on "open in new tab" in the top right
Open dev tools to see the hydration warnings in the console

Expected behavior
No hydration warning.
Screenshots or Videos
No response
Platform

OS: macOS
Browser: Chrome
Version: 1.7.5

Additional context
No response

If you are accessing children outside of a single location and for retrospection you should use the the children helper. Each lazy access recreates them and there are cases where that messes up hydration id matching. This issue is similar to this: https://github.com/solidjs/solid/issues/1485
In general you don't want to access props.children multiple times if you can avoid it SSR aside, but this accents it. A tradeoff of having things directly return the DOM elements.
For your specific case children has a toArray helper which I'm realizing isn't documented.
const c = children(() => props.children)
return (
    <For each={c.toArray()}>
      {(c, i) =>
        <>
          {/* add a divider */}
          {i() > 0 ?
 : null}
          {c}
        </>
      }
    </For>
  );

I've opened an issue: https://github.com/solidjs/solid-docs/issues/257
The rest of docs for children are here: https://www.solidjs.com/docs/latest/api#children

Ah yes I have played around with children() (and the toArray helper). After digging into it a bit more, turns out the issue was a bit more specific, and of course that "one extra bit of functionality" seems to be the culprit 😅. I'm allowing the consumer to optionally customize the divider, which introduces an extra conditional. This extra conditional seems to cause the hydration warning, but only sometimes.
interface StackProps {
  divider?: true | ValidComponent;
  children: JSX.Element;
}

const Stack = (props: StackProps) => {
  const c = children(() => props.children).toArray();

  return (
    <For each={c}>
      {(c, i) => (
        <>
          {/* optionally add a divider. default to
 w option to render custom component */}
          {i() > 0 && props.divider ? (
            props.divider === true ? (

            ) : (
              props.divider
            )
          ) : null}

          {c}
        </>
      )}
    </For>
  );
};

Usage
This works just fine! No hydration error, renders with the custom divider, etc:
const CustomDivider = () => {
  return "---";
};

<Stack divider={CustomDivider}>

one

two

</Stack>

This results in a hydration error:
<Stack divider>

one

two

</Stack>

Honestly at this point since it's so specific I can probably just remove the feature that allows for a custom divider to be supplied (thus removing the extra ? conditional that seems to be causing an issue), so it's not a big deal. Reporting though in case this is a symptom of some other bug..

Ok a weird twist - the hydration warning goes away if adjusted to use Show:
const Stack = (props: StackProps) => {
  const c = children(() => props.children).toArray();

  return (
    <For each={c}>
      {(c, i) => (
        <>
          {/* optionally add a divider. default to
 w option to render custom component */}
          <Show when={i() > 0 && props.divider}>
            {props.divider === true ?
 : props.divider}
          </Show>

          {c}
        </>
      )}
    </For>
  );
};

Hmm... well props.divider is potentially JSX too right.. It probably should to be wrapped in children when it is a custom option as you are accessing it multiple times. Why Show works is interesting. Ternary compilation does differ and it must be impacting element creation in a specific way.
I think what we are seeing is a combination of a couple things that usually don't happen/happen together. I think the answer here is to use children on props.divider but it may be worth looking at ternary execution. Honestly I think ultimately this is related to the issue I talked about in my previous response. Since I have a better answer there I will continue to track that there and close out this one. I'm glad you have a working solution.

No problem, working for now - thanks @ryansolid!


chore: update to stable stale action

closes: https://github.com/prisma/prisma/issues/3112

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


API v3: Processer les dossiers apprenants au nouveau format

:tada: This issue has been resolved in version 3.27.0 :tada:
The release is available on:

GitHub release
v3.27.0

Your semantic-release bot :package::rocket:


[INLONG-7537][Sort] Enable incremental snapshot for MongoDB CDC

Prepare a Pull Request
[INLONG-7537][Sort] Enable incremental snapshot for MongoDB CDC

Fixes #7537

Motivation
Explain here the context, and why you're making that change. What is the problem you're trying to solve?
After enabling incremental snapshots in MongoDB CDC, data can be read in chunks to improve reading efficiency.
Modifications
Describe the modifications you've done.
Add the MongoDBSource class to implement incremental snapshot logic.
Documentation

Does this pull request introduce a new feature? (yes)
If yes, how is the feature documented? ( docs )

This feature add some options, details for  https://github.com/apache/inlong-website/pull/710 .

Option
Required
Default
Type
Description

batch.size
optional
1024
Integer
The cursor batch size.

scan.incremental.snapshot.enabled
optional
false
Boolean
Whether enable incremental snapshot. The incremental snapshot feature only supports after MongoDB 4.0.

scan.incremental.snapshot.chunk.size.mb
optional
64
Integer
The chunk size mb of incremental snapshot.

chunk-meta.group.size
optional
1000
Integer
The group size of chunk meta, if the meta size exceeds the group size, the meta will be divided into multiple groups.

@healchow Hi, this PR https://github.com/apache/inlong/pull/7570 has been closed. I split it into two. Please review current PR.

@gong @EMsnap @yunqingmoswu pls help review it.


[BUG] vm-import-controller status keeps changing

Describe the bug

The vm-import-controller status changes between AddonEnabled and AddondeploySuccessful
To Reproduce
Steps to reproduce the behavior:

setting vm-import-controller to enabled equals true

Expected behavior

Support bundle

Environment

Harvester ISO version:
Underlying Infrastructure (e.g. Baremetal with Dell PowerEdge R630):

Additional context
Add any other context about the problem here.
https://user-images.githubusercontent.com/24985926/195018953-e1c1920f-d832-4c5a-9a76-bd1532d6003f.mov

@WuJun2016  I was able to reproduce this in the UI and also with calling describe on the resource using kubectl on the Harvester cluster.
Environment: qemu/kvm
Harvester Version: v1.1.0-rc2
Based around some tests it seems that more often it's shown as "AddonEnabled" than it is shown as "AddonDeploySuccessful":
#######################
####### SUMMARY #######
#######################
Total Number Of Times, Status was seen as AddonEnabled: 65

Total Number Of Times, Status was seen as AddonDeploySuccessful: 36

This is the small bash script I wrote to audit this with kubectl:
#! /bin/bash
ADDON_ENABLED_COUNT=0
ADDON_DEPLOY_SUCCESSFUL_COUNT=0
for i in {1..101};
do
  echo "on $i iteration - for checking addons.harvesterhci.io vm-import-controller status...";
  RESULT=$(kubectl describe addons.harvesterhci.io vm-import-controller -n harvester-system | grep -e "Status:")
  echo $RESULT;
  if [[ "$RESULT" == *"AddonEnabled"* ]]; then
    echo "found AddonEnabled as the status for addons.harvesterhci.io vm-import-controller";
    ADDON_ENABLED_COUNT=$((ADDON_ENABLED_COUNT+1));
  fi
  if [[ "$RESULT" == *"AddonDeploySuccessful"* ]]; then
    echo "found AddonDeploySuccessful as the status for addons.harvesterhci.io vm-import-controller";
    ADDON_DEPLOY_SUCCESSFUL_COUNT=$((ADDON_DEPLOY_SUCCESSFUL_COUNT+1));
  fi
  sleep 1;
  echo "current AddonEnabled Count of Number of times seen as status: $ADDON_ENABLED_COUNT";
  echo "current AddonDeploySuccessful Count of Number of times seen as status: $ADDON_DEPLOY_SUCCESSFUL_COUNT";
done
echo "#######################";
echo "####### SUMMARY #######";
echo "#######################";
echo "Total Number Of Times, Status was seen as AddonEnabled: $ADDON_ENABLED_COUNT";
echo "";
echo "Total Number Of Times, Status was seen as AddonDeploySuccessful: $ADDON_DEPLOY_SUCCESSFUL_COUNT";
echo "";

cc: @ibrokethecloud

This is fixed in  master builds.

This stays at "AddonDeploySuccessful" on Harvester Version: master-937f333c-head.
Tested for 5x what was previously tested.
500 runs. Status stays on correct value.
test_results_2905.log
This looks great!
Thanks for the fix @ibrokethecloud :smile:  - I'll go ahead and close this out :+1:


Hide chat window on startup

I'm seeing this warning about chat performance a lot

The warning allows the user to hide the chat window on startup. Ideally we'd be able to do that from the extension itself so the user doesn't see this warning. I looked into this earlier but couldn't figure out how to do it.

I don't think we could do that, as it's the users' choice to do so, FYI this message only shows up in the debug build and when the debugger is attached, it's still annoying though.
Also, the SetChatThemeAsync function is bugging out here, I've had that a couple of times, and I'm not sure whether the function was never called or the javascript was injected too late.


Mark as stable?

Now that we've eliminated what I think is most of the big issues (number validation comes to mind), can we mark this as stable?

yes I think so.
On Wed, Oct 26, 2011 at 8:12 PM, Alex Jordan <
<EMAIL_ADDRESS>
Now that we've eliminated what I think is most of the big issues (number
validation comes to mind), can we mark this as stable?
--
Reply to this email directly or view it on GitHub:
https://github.com/strugee/HelloWorld3/issues/7

--
Jackson de Campos

cool
On Wed, Oct 26, 2011 at 8:18 PM, jacksondc <
<EMAIL_ADDRESS>
yes I think so.
On Wed, Oct 26, 2011 at 8:12 PM, Alex Jordan <
<EMAIL_ADDRESS>
Now that we've eliminated what I think is most of the big issues (number
validation comes to mind), can we mark this as stable?
--
Reply to this email directly or view it on GitHub:
https://github.com/strugee/HelloWorld3/issues/7

--
Jackson de Campos
--
Reply to this email directly or view it on GitHub:
https://github.com/strugee/HelloWorld3/issues/7#issuecomment-2539273

CLOSING, CALCULATOR IS STABLE WHOOOOOOO!!

nice!
On Wed, Oct 26, 2011 at 9:10 PM, Alex Jordan <
<EMAIL_ADDRESS>
CLOSING, CALCULATOR IS STABLE WHOOOOOOO!!
--
Reply to this email directly or view it on GitHub:
https://github.com/strugee/HelloWorld3/issues/7#issuecomment-2539495

--
Jackson de Campos

I know right
On Oct 27, 2011, at 8:15 AM<EMAIL_ADDRESS>wrote:

nice!
On Wed, Oct 26, 2011 at 9:10 PM, Alex Jordan <
<EMAIL_ADDRESS>
CLOSING, CALCULATOR IS STABLE WHOOOOOOO!!
--
Reply to this email directly or view it on GitHub:
https://github.com/strugee/HelloWorld3/issues/7#issuecomment-2539495

--
Jackson de Campos
--
Reply to this email directly or view it on GitHub:
https://github.com/strugee/HelloWorld3/issues/7#issuecomment-2544751


letsencrypt: fix clearing of hosts list

This fixes a problem where all connections are refused with the "unknown hosts" error if Manager.SetHosts is called with nil  or an empty list.
This fix is required to be able to clear a hosts list that was loaded from the cache file.

Quoting the godoc:

In the long term you should be using https://golang.org/x/crypto/acme/autocert instead of this package. Send improvements there, not here.

Please use autocert instead of this package. This package is frozen. Thanks.


Allow full error trace in Nunjucks

When a global function in a template throws an error, we don’t see the full trace of the error, which makes diagnosing issues really hard.
This makes use of the undocumented dev argument which allows us to see the cause of the error in our logs and in Sentry.
Copied from CAS1.
Context

Changes in this PR
Screenshots of UI changes
Before
After
Release checklist
As part of our continuous deployment strategy we must ensure that this work is
ready to be released at any point. Before merging to main we must first
confirm:
Pre merge checklist

[ ] Are any changes required to the e2e tests?
[ ] If you've added a new route, have you added a new
auditEvent? (see server/routes/apply.ts for examples)
[ ] Are there environment variables or other infrastructure configuration which needs to be included in this release?
[ ] Are there any data migrations required. Automatic or manual?
[ ] Does this rely on changes being deployed to the CAS API?

Post merge checklist
Once we've merged it will be auto-deployed to the dev environment.
Find the build-and-deploy job in CircleCI.
Should a release fail at any step, you as the author should now lead the work to
fix it as soon as possible.

I see this is what CAS1 did but I'm feeling cautious. Configuring an app that's running in production to behave like dev might have other unexpected consequences.
I'm wary that this might somehow log too much info to the client or for any other part of Nunjucks to have plugged into this config (based on the generic name) which might cause some other dev only behaviour.
The comment within nunjucks does suggests that all it does is change logging though.. I tracked the origin way back and it does seem to have been added with the explicit purpose of improving error clarity. And CAS1 have used it and no weird behaviour has been found yet. So on balance this is probably ok as everyone is a signed in user.
I ran out of time to try and observe better stack traces locally because ironically my stack seemed broken but I assume it works.

Good points. I guess on this basis we can try it and see how it goes.
It does work - I should have added some screenshots but will now.


Pydantic compatibility issue

There is a compatibility issue when other projects use Pydantic in addition to the Shazamio library.
Please update compatibility with new libraries, because shazamio uses very outdated versions of libraries.

shazamio <IP_ADDRESS> requires pydantic<2.0.0,>=1.10.2, but you have pydantic 2.5.3 which is incompatible.

@D2PF3 Hello!
If you can help with this - I will be grateful! I'm currently developing core in rust.

Hi, any news on this?
I'm trying to include shazamio in a buildozer/kivy project, since the pydantic recipe points to 2.16.1, shazamio cannot work.
On android phone logcat I have:
 File "/home/jenkins/workspace/nization_folder_musenote_develop/.buildozer/arm64-v8a/android/platform/build-arm64-v8a/build/python-installs/musenote/arm64-v8a/shazamio/__init__.py", line 1, in <module>
   File "/home/jenkins/workspace/nization_folder_musenote_develop/.buildozer/arm64-v8a/android/platform/build-arm64-v8a/build/python-installs/musenote/arm64-v8a/shazamio/serializers.py", line 3, in <module>
   File "/home/jenkins/workspace/nization_folder_musenote_develop/.buildozer/arm64-v8a/android/platform/build-arm64-v8a/build/python-installs/musenote/arm64-v8a/shazamio/schemas/base.py", line 20, in <module>
   File "/home/jenkins/workspace/nization_folder_musenote_develop/.buildozer/arm64-v8a/android/platform/build-arm64-v8a/build/python-installs/musenote/arm64-v8a/pydantic/_internal/_model_construction.py", line 115, in __new__
   File "<frozen abc>", line 106, in __new__
 TypeError: Cannot create a consistent method resolution
 order (MRO) for bases BaseModel, BaseIdTypeHref, Generic

I already reported this to python4android project here: https://github.com/kivy/python-for-android/issues/3024 thinking was pydantic_core issue.
Let me know if you need further information or if there is something I should try.
Best regards,
magowiz

@D2PF3 Hello! If you can help with this - I will be grateful! I'm currently developing core in rust.

Since this project already uses poetry, I believe it should be enough to run poetry update pydantic.


 Are you opening this pull request for bug-fixs, optimizations or new feature?
optimizations
First of all, thanks to ants for maintaining a good pool implementation. In the process of using it, it was found that some goroutines did not exit when Release was called. This mr change will cause all goroutines to exit immediately.
Except for the first time, the global defaultAntsPool will be created by default. In this mr, defaultAntsPool will be created the first time it is called.
2. Please describe how these code changes achieve your intention.

TODO
3. Please link to the relevant issues (if any).

TODO
4. Which documentation changes (if any) need to be made/updated because of this PR?

TODO
4. Checklist

[x] I have squashed all insignificant commits.
[x] I have commented my code for explaining package types, values, functions, and non-obvious lines.
[x] I have written unit tests and verified that all tests passes (if needed).
[x] I have documented feature info on the README (only when this PR is adding a new feature).

It seems that you're trying to make the goroutine of purgePeriodically quit once the pool.Release() is called, however, what's wrong about this goroutine quitting only after all other worker goroutines quit?


feat(playground): add custom css panel

close #2181
You can simply preview it.

The url length is really long.

Can we have tabs instead? A toggle might be too unnoticeable. Like
> | Output CSS | Custom CSS |  <actions>

Can we have tabs instead? A toggle might be too unnoticeable. Like
> | Output CSS | Custom CSS | <actions>

Can use Output | Custom instead? The title is too long and not friendly to small screens.

Or maybe just one more panel?

Added new change:

[x] Add Custom CSS panel before Output CSS panel.
[x] Strip the <Headerbar /> from the <HTMLpanel />
[x] Fixed get the splutpanes element height is undefined

The url length is really long.

Should not exceed 2048 characters, but (I0ve never tested it :sweat_smile: ):

Google Chrome: allows the maximum length of the URL to be of the size 2MB (2,097,152 characters).
Mozilla Firefox: the length of the URL can be unlimited but practically after 65,536 characters the location bar no longer displays the URL.
Edge: allows the URL length to be a maximum of 2083 characters but not more than 2048 characters in the path portion of the URL.
Opera: allows the URL length to be unlimited.
Safari: maximum length of URL in Safari is 80000 characters after crossing this limit the page displays an error.

The url length is really long.

Should not exceed 2048 characters, but (I've never tested it 😅 ):

Google Chrome: allows the maximum length of the URL to be of the size 2MB (2,097,152 characters).
Mozilla Firefox: the length of the URL can be unlimited but practically after 65,536 characters the location bar no longer displays the URL.
Edge: allows the URL length to be a maximum of 2083 characters but not more than 2048 characters in the path portion of the URL. New version should have the same limit Chrome has (Chromiun)
Opera: allows the URL length to be unlimited.
Safari: maximum length of URL in Safari is 80000 characters after crossing this limit the page displays an error.

Sometimes, i want to share some unocss playground demo on Elk or twitter, because of that length is too long, i have to abandoned it.😅


if-else statement should use opposite test and fix semicolon placement

https://github.com/apayne/supervision-scripts/blob/a38c7795374539a1fe6c08d7cfe8c05e18e0c71b/svcdef/.log/autolog#L22
should be:
if test -n $LOGSV; then

You are right about the fixes.
After making these changes, does it work for you @jvvv ?
I get an error:
./run: 23: exec: ./run-svlogd: not found

Perhaps exec should be removed?.. ie:
./run-$LOGSV

It appears that the autolog script can't find the file.  The file name is correct, but the path is relative to where autolog is installed.  Remember, the dot directories need to be installed in the same location as the running copies of the service definitions; those definitions pull from their local directory, not from where /svcdef is installed.
(The idea was that the scripts and directories were to be portable to any location on the filesystem because there was no way in advance to know what the filesystem would be strutured like, so relative pathing to where the installation and scripts are run was important.)
For example, let's say you have /etc/svcdef holding the definitions from this repository, /etc/svc holding the active definitions, and /service is a softlink pointing to /etc/svc.  You would want to make sure that the .env, .finish, .log and .run directories are coped from /etc/svcdef to /etc/svc; all of the active scripts run from /etc/svc and will call pathing relative to that, allowing the logging to be found at /etc/svc/.log

A follow-up: I went back and took a peek at the last commits, as I haven't done much with the project in a few years.  I noticed that autolog is deprecated in a commit, and I reverted back to having the person doing the install manually select the logging.  While you can use it to attempt to automatically set up your environment, it's not really supported at this time.  If it can be made to work consistently, great, we'll expand it out.

Sorry that I haven't had time to revisit this. I really want to make an attempt to start using supervision-scripts, but my work hours have been so heavy it has been problematic for me. Still, I like the approach and hope to find the time eventually. Thanks for sharing this!


Application permission check on certain commands

Originating from https://github.com/pnp/cli-microsoft365/issues/3894#issuecomment-1307805497
We could and maybe should add extra checks since some commands don't work with application permissions.
Right now we are already doing this for planner commands.
https://github.com/pnp/cli-microsoft365/blob/1cd4e45a4596b6bee06b976a26ee95ee90dac81c/src/m365/planner/commands/bucket/bucket-add.ts#L108-L111
Should we maybe create a central function that checks this rather than copy-pasting these lines of code over and over again?
Commands to update

[ ] All flow commands
[ ] All Power Apps commands
[ ] All Power Platform commands
[ ] All Yammer commands
[ ] teams chat send
[ ] ...

Let's check for other commands where this could be useful as well.

@pnp/cli-for-microsoft-365-maintainers what do you think?

Good idea to add it to all commands where it's applicable. I wonder if we can centralize it though, since we need to break function execution by calling return on the main flow.

I wonder if we can centralize it though, since we need to break function execution by calling return on the main flow.

Good point, right we do a return indeed, but that was because we used to work with callbacks. When we passed an error to the callback function and had to add a return statement to prevent it from executing more of the commandAction.
However, we are now working with promise/await. We are now throwing errors, when we throw an error, the function will be terminated as well, so actually we can remove the return right now since it will never be hit anyway.

Good point! If we can use throw to break the code flow, it would suffice as well!

Just not sure if we should centralise a single if  with 1 line of code in the body. Or should we just copy/paste it.
Might be overkill to centralise 3 lines of code.

There's little overhead when it comes to centralizing it and offers us a great deal of consistency especially when it comes to the logic we use to decide if we're in app-only auth and the error message we return. I'm all for centralizing.

@milanholemans I would love to give this a go. Would it be an idea to write this logic if all the commands of a single set require the check to add it in the command that it command that it extends, so for example for PowerPlatform, in the PowerPlatformCommand class?
I'll also do some research on other commands that require this check.

Not sure about this. While this would work for PowerPlatform commands, for example, we also have a bunch of commands that support both delegated & application permissions. Take a Graph command for example. Most commands support delegated and application permissions, but there are some commands that don't support application permissions.
I think we should work uniformly and use the same logic everywhere.
Maybe we could make a util class that checks if the command is running in app only mode and throws an error if this is the case. This will prevent us from copy-pasting the same code over and over again.

Not sure about this. While this would work for PowerPlatform commands, for example, we also have a bunch of commands that support both delegated & application permissions. Take a Graph command for example. Most commands support delegated and application permissions, but there are some commands that don't support application permissions.
I think we should work uniformly and use the same logic everywhere.

Of course I would only do this on where the entire set of commands use this, not where a Graph API is being used as this is command-specific.

Maybe we could make a util class that checks if the command is running in app only mode and throws an error if this is the case. This will prevent us from copy-pasting the same code over and over again.

Makes sense to write this in a util. Would we allow the user to pass a custom error message? Sometimes we will only block application permissions when updating specific properties, so we wouldn't have to throw a generic error. Or would we just retain the old way for these cases?

Sometimes we will only block application permissions when updating specific properties, so we wouldn't have to throw a generic error.

I'd say in these very edge cases, we just write our own logic in the command.

@MathijsVerbeeck thinking about it some more. Stuff like Power Platform doesn't allow app permissions at all for any command, so maybe it does make sense to put it in the base command.

I think that all the todo commands also require this check, as they call the me andpoint


[Event Change Request] Codeunit 826 "Purch. Post Invoice Events" - RunOnCalculateVATAmountsOnAfterGetReverseChargeVATPostingSetup

Describe the request
Hello, could you please add the Parameter "TempInvoicePostBuffer" to the Procedure RunOnCalculateVATAmountsOnAfterGetReverseChargeVATPostingSetup in Codeunit 816 "Purch. Post Invoice" and to the Event OnCalculateVATAmountsOnAfterGetReverseChargeVATPostingSetup in the Codeunit 826 "Purch. Post Invoice Events"
procedure RunOnCalculateVATAmountsOnAfterGetReverseChargeVATPostingSetup(var VATPostingSetup: Record "VAT Posting Setup")
    begin
        OnCalculateVATAmountsOnAfterGetReverseChargeVATPostingSetup(VATPostingSetup);
    end;

    [IntegrationEvent(false, false)]
    local procedure OnCalculateVATAmountsOnAfterGetReverseChargeVATPostingSetup(var VATPostingSetup: Record "VAT Posting Setup")
    begin
    end;

Eventsubscriber in "Old" Purch. Post Codeunit:
#if not CLEAN23
    [Obsolete('Moved to Purchase Invoice Posting implementation. Use the new event OnCalculateVATAmountsOnAfterGetReverseChargeVATPostingSetup in codeunit 826 "Purch. Post Invoice Events".', '20.0')]
    [IntegrationEvent(false, false)]
    local procedure OnPostInvoicePostingBufferOnAfterVATPostingSetupGet(var VATPostingSetup: Record "VAT Posting Setup"; var TempInvoicePostBuffer: Record "Invoice Post. Buffer" temporary; var IsHandled: Boolean)
    begin
    end;
#endif

Additional context
This was already the Case in the Old Purch.- Post Codeunit.
We would like to change the Vat% in the Posting buffer to change the Reverse charge VAT for Swiss Billing to abroad companies.
Internal work item: AB#555755

Availability update: We will publish a fix for this issue in the next update for release 25.
Build ID to track: 26491.


investigate innodb compression

http://dev.mysql.com/doc/refman/5.5/en/innodb-compression.html
Original issue by @bmesuere on Mon Mar 16 2015 at 15:14.

A few small tests on my own laptop using only swissprot:

the fileformat must be set to barracuda
enabling it on the genome_caches table reduced the table size from 71MB to 23MB
enabling it on the uniprot_entries table reduced the data size from 274MB to 154MB and the index size from 7.8MB to 3.9MB
unrelated, but dropping the unused position column from peptides, reduced the data size from 471MB to 460MB

Original comment by @bmesuere on Fri Apr 03 2015 at 18:28.


update plugins to Jenkins 2

In this pull request I would like contribute with the project to update the latest stable versions of this list of plugins.

ok all the validations / manual regression testing from me is good
if @firemanxbr could add that one comment I asked for in https://github.com/openshift/jenkins/pull/502/files#r167982184 I'll merge this
thanks for the contribution!

@grdryn @gabemontero thanks for the review guys. I added the comment requested. If I can improve more somethings please let me know.

Looks great @firemanxbr - thanks!
Was about to merge but then ...
I wonder if our bot is broke ... we had to update the openshift-sync and openshift-client versions (1.0.3 and 1.0.7) for some end of release bug fixes ... but I don't see a notification that you have to rebase.    So a rebase is needed.
@stevekuznetsov @dobbymoodge @bparees any ideas?
@firemanxbr in the meantime could you rebase on the latest master, pick up commit b27d611ef0e3723c79d2008e929772f8bdfcb5e9? ... sorry for the churn

Our bots use GitHub's rebase status to trigger the messages, and the GitHub UI is not showing a rebase as necessary.

@gabemontero github would flag the PR if there was a merge conflict.  If there's no merge conflict, rebasing is handled automatically.

(and by "rebasing is handled automatically" i mean, the commits will be merged on top of our master, which has the appropriate sync plugin version)

@gabemontero now, for testing purposes, this is, I believe, one of the repos where the CI system just tests the PR branch, not "the PR commits merged onto master", so if you're worried about ensuring this PR gets tested using the right version of the sync plugin, yes the PR will need to be rebased.

@gabemontero I used the rebase to keep the commit logs clean(stash). In my team at Red Hat we have this procedure :D

thanks everyone ... and based on my previous testing, we should go good re: regression testing

[merge]

new plugin list:
ace-editor:1.0.1
authentication-tokens:1.1
blueocean-autofavorite:1.2.0
blueocean-bitbucket-pipeline:1.4.1
blueocean-commons:1.4.1
blueocean-config:1.4.1
blueocean-core-js:1.4.1
blueocean-dashboard:1.4.1
blueocean-display-url:2.1.1
blueocean-events:1.4.1
blueocean-github-pipeline:1.4.1
blueocean-git-pipeline:1.4.1
blueocean-i18n:1.4.1
blueocean-jira:1.4.1
blueocean:1.4.1
blueocean-jwt:1.4.1
blueocean-personalization:1.4.1
blueocean-pipeline-api-impl:1.4.1
blueocean-pipeline-editor:1.4.1
blueocean-pipeline-scm-api:1.4.1
blueocean-rest-impl:1.4.1
blueocean-rest:1.4.1
blueocean-web:1.4.1
branch-api:2.0.11
cloudbees-bitbucket-branch-source:2.2.5
cloudbees-folder:6.1.2
conditional-buildstep:1.3.1
config-file-provider:2.17
credentials-binding:1.15
credentials:2.1.15
display-url-api:2.1.0
docker-commons:1.11
docker-workflow:1.9
durable-task:1.16
favorite:2.3.1
git-client:2.5.0
github-api:1.86
github-branch-source:2.2.3
github:1.26.0
github-organization-folder:1.6
git:3.6.0
git-server:1.5
handlebars:1.1
htmlpublisher:1.14
jackson2-api:2.7.3
jenkins-design-language:1.4.1
jira:2.4.2
job-dsl:1.68
jquery-detached:1.2.1
jsch:<IP_ADDRESS>
junit:1.23
kubernetes-credentials:0.3.0
kubernetes:1.2
mailer:1.20
mapdb-api:<IP_ADDRESS>
matrix-auth:2.2
matrix-project:1.12
mercurial:2.2
momentjs:1.1
openshift-client:1.0.7
openshift-login:1.0.4
openshift-pipeline:1.0.54
openshift-sync:1.0.3
parameterized-trigger:2.35.2
pipeline-build-step:2.7
pipeline-github-lib:1.0
pipeline-graph-analysis:1.6
pipeline-input-step:2.8
pipeline-milestone-step:1.3.1
pipeline-model-api:1.2.5
pipeline-model-declarative-agent:1.1.1
pipeline-model-definition:1.2.5
pipeline-model-extensions:1.2.5
pipeline-rest-api:2.9
pipeline-stage-step:2.2
pipeline-stage-tags-metadata:1.2.5
pipeline-stage-view:2.4
pipeline-utility-steps:1.5.1
plain-credentials:1.3
pubsub-light:1.12
run-condition:0.10
scm-api:2.2.2
script-security:1.41
sse-gateway:1.15
ssh-credentials:1.13
structs:1.13
subversion:2.10.2
token-macro:2.0
variant:1.1
workflow-aggregator:2.5
workflow-api:2.22
workflow-basic-steps:2.3
workflow-cps-global-lib:2.7
workflow-cps:2.42
workflow-durable-task-step:2.13
workflow-job:2.14.1
workflow-multibranch:2.16
workflow-remote-loader:1.4
workflow-scm-step:2.4
workflow-step-api:2.14
workflow-support:2.14

update plugin rpm for rhel:  https://buildvm.openshift.eng.bos.redhat.com:8443/job/devex/job/devex%252Fjenkins-plugins/28/

and yep it git merged the changes just fine .... the latest openshift-* versions are there


Question: Warning: importing 'simtk.openmm' is deprecated

I have finished my first run with colabfold. Everything seems to work and I get output files, including pdbs for all sequences referenced. However, at the very beginning of the run, I got this message: Warning: importing 'simtk.openmm' is deprecated.  Import 'openmm' instead.
Am I good to continue?

Yes, the warning message does not harm the calculation. I can suppress the message but haven't dealt with it yet.


Try again to fix INFRA-15650

This needs more work to enable the AuthzProviderAlias section.
However it should be usable as is (except ASF members won't get access to the /pmc area).
But at least PMC members will have access to the /pmc area

This is fine by me, what we end up with currently is still an marked improvement on mail-search as it was.


Downloading Youtube private video with cookiefile no longer works

Make sure you are using the latest version: run youtube-dl --version and ensure your version is 2017.09.11. If it's not, read this FAQ entry and update. Issues with outdated version will be rejected.

[x] I've verified and I assure that I'm running youtube-dl 2017.09.11

Before submitting an issue make sure you have:

[x] At least skimmed through the README, most notably the FAQ and BUGS sections
[x] Searched the bugtracker for similar issues including closed ones

What is the purpose of your issue?

[x] Bug report (encountered problems with youtube-dl)
[ ] Site support request (request for adding support for a new site)
[ ] Feature request (request for a new functionality)
[ ] Question
[ ] Other

If the purpose of this issue is a bug report, site support request or you are not completely sure provide the full verbose output as follows:
Add the -v flag to your command line you run youtube-dl with (youtube-dl -v <your command line>), copy the whole output and insert it here. It should look similar to one below (replace it with your log inserted between triple ```):
[debug] System config: []
[debug] User config: []
[debug] Custom config: []
[debug] Command-line args: ['-v', '--cookies', 'cookies.txt', 'https://youtu.be/sdYucbUu_sw']
[debug] Encodings: locale UTF-8, fs utf-8, out UTF-8, pref UTF-8
[debug] youtube-dl version 2017.09.11
[debug] Python version 3.6.2 - Linux-4.12.12-1-ARCH-x86_64-with-arch
[debug] exe versions: ffmpeg 3.3.3, ffprobe 3.3.3, rtmpdump 2.4
[debug] Proxy map: {}
[youtube] sdYucbUu_sw: Downloading webpage
[youtube] sdYucbUu_sw: Downloading embed webpage
[youtube] sdYucbUu_sw: Refetching age-gated info webpage
ERROR: sdYucbUu_sw: YouTube said: This video is private.
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/youtube_dl/YoutubeDL.py", line 776, in extract_info
    ie_result = ie.extract(url)
  File "/usr/lib/python3.6/site-packages/youtube_dl/extractor/common.py", line 434, in extract
    ie_result = self._real_extract(url)
  File "/usr/lib/python3.6/site-packages/youtube_dl/extractor/youtube.py", line 1608, in _real_extract
    expected=True, video_id=video_id)
youtube_dl.utils.ExtractorError: sdYucbUu_sw: YouTube said: This video is private.

Description of your issue, suggested solution and other information
My university hosts webcast lectures on Youtube, but requires being logged in as the correct user. In the past I have been able to use a cookies.txt file (from this Chrome extension) to download the video through youtube-dl. However I am now given the error: "This video is private."

After some experimentation I've determined that this is not a youtube-dl bug. Using this script with Firefox I have been able to download videos again.


MGDOBR-1294: Refactor TemplateProvider in v2

https://issues.redhat.com/browse/MGDOBR-1294
PR owner checklist
Please make sure that your PR meets the following requirements:

[x] Your code is properly formatted according to this configuration.
Run mvn clean verify -DskipTests so that imports are correctly set.
[x] Your commit messages are clear and reference the JIRA issue e.g: "[MGDOBR-XXX] - $clear_explanation_of_what_you_did"
[x] The layers in the kustomize folder have been updated accordingly.
[x] All new functionality is tested
[x] Pull Request title is properly formatted: MGDOBR-XYZ Subject
[x] Pull Request contains link to the JIRA issue
[x] Pull Request contains link to any dependent or related Pull Request
[x] Pull Request contains description of the issue
[x] Pull Request does not include fixes for issues other than the main ticket

PR reviewer(s) checklist

[ ] Check if code and Github action workflows have been modified and if the modification is safe
[ ] If the modification is safe, add the safe to test label

How to trigger pipelines and use the bots:

Run the end to end pipeline
Annotate the pull request with the label: safe to test. If you want to run the pipeline again, remove and add it again.

Rebase the pull request
Comment with: /rebase.

Deploy BOT
Comment with `/deploy <dev,stable> when the PR has been merged to deploy to a target environment.

Thanks Michael for the PR, it looks good
What I wanted to see is to avoid every trace of thenCallRealMethod()
        when(templateProvider.loadBridgeExecutorSecretTemplate(any(), any())).thenCallRealMethod();
In BridgeExecutorServiceTest and the other similar tests.
do you think it could be possible with this PR?

Hi @lucamolteni

I wanted to ... avoid every trace of thenCallRealMethod()
 when(templateProvider.loadBridgeExecutorSecretTemplate(any(), any())).thenCallRealMethod();
In BridgeExecutorServiceTest and the other similar tests.
do you think it could be possible with this PR?

I wasn't planning on re-writing v1 tests to be honest. I could, if you insist, but I wasn't planning on it.
It is however probably good to communicate the anti-pattern of using thenCallRealMethod(..) ongoing.

One of the reason of this refactor is, a part from the obvious coupling of the Template with v1 templates, is that every new method added to the TemplateProviderV1 will require those test to be changed with an explicit call to thenCallRealMethod() otherwise the test will fail with an unrelated error message.
Now with this split I think we're safe since we won't be adding more methods to TemplateProviderV1 (hopefully) and we won't hit that problem anymore so... up to you :)

@lucamolteni

One of the reason of this refactor is, a part from the obvious coupling of the Template with v1 templates, is that every new method added to the TemplateProviderV1 will require those test to be changed with an explicit call to thenCallRealMethod() otherwise the test will fail with an unrelated error message.

I checked adding new methods to TemplateProviderV1 and they do not break the tests.
The methods marked for thenCallRealMethod() are on a very specific code-path used by the tests themselves.


Additional documentation for contour.py example

I have been using mplleaflet successfully in other contexts (and it's great!) but just wanted to suggest a small modification (or additional documentation to) the contour.py so that new users like myself can reproduce the results.
I (and independently a friend) have not been able to get the contour.py example to work.  We loaded rain data from the listed source into the geodataframe and got matplotlib.contour plot to correctly display in an ipython notebook. However when we make the last mplleaflet.show call the browser window displays only the basemap with no sign of the contour plot.
The contour.py module does mention that the CRS info has been only 'gleaned from reading the NWS and Proj.4 docs' but someone seems to have made it work to produce the example map that you link to in the README.
The data we downloaded is more recent precipitation data than in the example (the last file I downloaded was "nws_precip_year2date_observed_20150101.shp"  whereas in the example it is "'nws_precip_year2date_observed_20140406.shp"). I don't think the CRS would change but perhaps it has. I can't see what else we are doing wrong since we are following the contour.py code practically verbatim.
So my suggestion is to either add documentation to give users more guidance on how to 'glean' the CRS themselves (if that is the problem) or perhaps easier and more effective: modify the example to work with another fixed dataset (with a known CRS) to be sure it works everytime.  Thanks.

Hi @jhconning. I'm glad you've found mplleaflet useful so far.
Recently a similar bug was discovered with the contour example - see #19 - and was fixed just a few days ago. Do you know if you're still seeing this behavior with the current master or latest update on PyPI (v0.0.2)? If you get a chance, can you try that out and report back?
I think the CRS is unrelated. Either way, I agree about that the doc is lacking there. I'll see if I can find more detailed descriptions for how I came up with the Proj4 parameters.

Thank your response.  I updated to v0.02 as you suggested via pip, but still no luck
Possibly related or another issue: On the final mplleaflet.show call in contour.py I omitted the 'tiles=' option because it was giving an error.  I assumed that leaving it blank would revert to a default tile option (appears to be 'maptiles.osm') which would display. This is pointing to something that is not put in place at the time of installation.  When I make the mplleaflet call exactly as it appears in contour.py example I get the following error
In[]: mplleaflet.show(crs=crs, path=mapfile, tiles='mapbox bright')
C:\Users\Jonathan\Anaconda3\lib\site-packages\mplleaflet\_display.py in fig_to_html(fig, template, tiles, crs, epsg)
     58     if tiles is None:
     59         tiles = maptiles.osm
---> 60     elif isinstance(tiles, basestring):
     61         if tiles not in maptiles.tiles:
     62             raise ValueError('Unknown tile source "{}"'.format(tiles))

NameError: name 'basestring' is not defined

FYI: I'm on Windows 7, using Anaconda (updated this AM) in a python 3.4 environment.

@jhconning Finally found some time to take a deeper look at this again.
I just downloaded a fresh file from http://water.weather.gov/precip/download.php for precipitation over the last 30 days. I changed the filename in the script to use the latest file.
It looks like the columns have changed casing. This caused a crash in the code. I just pushed a fix forcing all columns to lowercase and then using those. I'm pretty sure this isn't your problem since you wouldn't have gotten to the point that the map tried to display.
Other than that, the script worked without any modification and the map displayed properly. The CRS for the HRAP system shouldn't depend on the specific file. I also double checked and tested on Python 2.7 and 3.3.
If you run the examples/contour.py script, changing only the filename, do you still have problems? You mentioned that you were trying to display it in the IPython notebook, so there could be problems there. Note that to embed in the notebook you have to call display(), not show().

The file I downloaded is nws_precip_last30days_observed_shape_20150823


Allow raylib to be built with zig 0.11.0 and zig 0.12.0dev (master)

Tested on linux, shouldn't cause any problems for other operating systems

@freakmangd thanks for the review, please note that I'm not validating this PR, hopefully some Zig users could confirm it works as expected and nothing breaks.


Sync settings.json on wsl2

Can the remote setting.json of wsl2 be synchronized?

This setting file is too old
😃most of my settings are on the wsl server

/duplicate

@sandy081
Which issue is it duplicated? I can't find related issues.

Sync vscod-server setting.json

https://github.com/microsoft/vscode/issues/90968

ok..i see


Search responds really slow

When typing, it takes a second or two for anything you type to actually respond.

I think maybe limiting the number of bps returned in that list to something like 80 or a 100 will help with this.

Search should be a lot more responsive in 0.1.6


Pattern Analysis

Adds functionality to analyze the minimum and maximum # of
characters a regex may match.

I don't understand motivation of the PR. What's the use-case?

In my use case I'm using it to categorize regexes at compile time into two sets, ones which may or may not consume characters when they succeed. I'm using your library with a c++ library taocpp pegtl which also mimics regular expression rules but to construct more complex. You can try to analyze the grammar for problems, say for example if you've created a grammar which contains an infinite loop, but in order to work that out their needs to understand the rule under one of four categories, one of the two I've described and two others, a rule that behaves like a an alternation or a rule that behaves like a sequence. The two later rules will eventually boil down into one of the top two.
The other use case would be rejecting input strings which are too short. If an input string for example was 10 characters long but we know the regex requires a bare minimum of 11, then we can reject the result at the start as opposed to processing all the rules. Rewriting the evaluation function a bit with an extra control struct at the start you could take these results and perform a size check on the start and end iterators.

I didn't consider variable length encodings before, looking back here's a better sketch of what'd be possible:
// calling with pattern prepare stack and triplet of iterators
template <typename Iterator, typename EndIterator, typename Pattern>
constexpr inline auto match_re(const Iterator begin, const EndIterator end, Pattern pattern) noexcept {
	using return_type = decltype(regex_results(std::declval<Iterator>(), find_captures(pattern)));
	if constexpr (!ctre::is_variable_length_encoded<Iterator>() && std::is_same<std::iterator_traits<Iterator>::iterator_category, std::random_access_iterator_tag>::value) {
		constexpr auto lengths = ctre::pattern_match_minmax_characters(ctll::list<start_mark, Pattern, assert_end, end_mark, accept>(), return_type());
		//check the size of the input string
		auto length = std::distance(begin, end);
		if (length >= lengths.min && length <= lengths.max) //if not within bounds we can avoid this call
			return evaluate(begin, begin, end, return_type{}, ctll::list<start_mark, Pattern, assert_end, end_mark, accept>());
		else
			return return_type{};
	} else {
                //normal unchecked size call
		return evaluate(begin, begin, end, return_type{}, ctll::list<start_mark, Pattern, assert_end, end_mark, accept>());
	}
}

template <typename Iterator, typename EndIterator, typename Pattern>
constexpr inline auto starts_with_re(const Iterator begin, const EndIterator end, Pattern pattern) noexcept {
	using return_type = decltype(regex_results(std::declval<Iterator>(), find_captures(pattern)));
	if constexpr (!ctre::is_variable_length_encoded<Iterator>() && std::is_same<std::iterator_traits<Iterator>::iterator_category, std::random_access_iterator_tag>::value) {
		constexpr auto lengths = ctre::pattern_match_minmax_characters(ctll::list<start_mark, Pattern, end_mark, accept>(), return_type());
		//check the size of the input string
		auto length = std::distance(begin, end);
		if (length >= lengths.min) //we only check the minimum size requirement since starts with implicitly trails w/ (.*) meaning the max is infinite
			return evaluate(begin, begin, end, return_type{}, ctll::list<start_mark, Pattern, end_mark, accept>());
		else
			return return_type{};
	} else {
                //normal unchecked size call
		return evaluate(begin, begin, end, return_type{}, ctll::list<start_mark, Pattern, end_mark, accept>());
	}
}

template <typename Iterator, typename EndIterator, typename Pattern>
constexpr inline auto search_re(const Iterator begin, const EndIterator end, Pattern pattern) noexcept {
	using return_type = decltype(regex_results(std::declval<Iterator>(), find_captures(pattern)));

	constexpr bool fixed = starts_with_anchor(ctll::list<Pattern>{});
	if constexpr (!ctre::is_variable_length_encoded<Iterator>() && std::is_same<std::iterator_traits<Iterator>::iterator_category, std::random_access_iterator_tag>::value) {
		constexpr auto lengths = ctre::pattern_match_minmax_characters(ctll::list<start_mark, Pattern, end_mark, accept>(), return_type());
		//check the size of the input string
		auto length = std::distance(begin, end);

		auto it = begin;
		for (; end != it && !fixed && length >= lengths.min; ++it) { //similar to starts_with, but we loop
			if (auto out = evaluate(begin, it, end, return_type{}, ctll::list<start_mark, Pattern, end_mark, accept>())) {
				return out;
			}
			length--;
		}

		// in case the RE is empty or fixed
		return evaluate(begin, it, end, return_type{}, ctll::list<start_mark, Pattern, end_mark, accept>());
	}
	else {
                //normal unchecked size loop
		auto it = begin;
		for (; end != it && !fixed; ++it) {
			if (auto out = evaluate(begin, it, end, return_type{}, ctll::list<start_mark, Pattern, end_mark, accept>())) {
				return out;
			}
		}

		// in case the RE is empty or fixed
		return evaluate(begin, it, end, return_type{}, ctll::list<start_mark, Pattern, end_mark, accept>());
	}
}

The const analysis_results min_max_range = trampoline_analysis(ctll::list<Pattern>(), return_type{}); from before would be ctre::pattern_match_minmax_characters<...>()

Had a thought*, pattern analysis can allow you to perform these transformations:
ctll::list<assert_begin, consuming sequence, assert_begin, Content...> -> ctll::list<reject>
ctll::list<assert_end, consuming sequence, assert_end, Content...> -> ctll::list<reject>

You'd only need to prove some sequence between the anchors consumes a minimum of 1 character, which the algorithm can provide.

Mind that every such analysis / transformation is instantiating a large amount of templates, and has a big impact on compile time.
Also optimizers can see a lots of thing already:
https://compiler-explorer.com/z/MoYhff

I guess that optimization should be expected since it would likely track the value of the pointer and work out that it would be incremented later.
It just strikes me how much compile time cost you're paying for having to do this with templates when for the most part regex's are relatively straightforward and probably could be made into byte code, passed to a compiler, optimized etc... and likely would give you back the same results. Say if someone were particularly crazy and made a frontend specifically for regex to llvm and have it build out a function per regex. Doesn't work out in terms of having something native for c++. But if there were an ability to generate bytecode that the compiler could take and use to define a function a lot of this would probably be a lot less painful.
Really wish you weren't fighting compile time costs, because in terms of writing optimizations you've made it pretty easy, even if the compiler's showing me up by working those out in the end anyway. Like here: https://github.com/hanickadot/compile-time-regular-expressions/issues/72 you mention (a*)* I'm fairly certain can be safely reduced to (a*). Being able to just write an overload to a function to do that is incredibly handy.


when checkAll, the checkboxs are not selected

When I click uncheckAll button, $scope.selectedItems is empty, but all the checkboxs are still selected. Is this because the id is not Number?
my data is like this:
$scope.data = [{
id: "234dfsdw43",
name: "name1"
},
{
id: "e4w54gr4",
name: "name2"
}]

JS:
$scope.checkAll = function() {
 $scope.selectedItems = $scope.data.map(function(item) { return item.id; });
};

$scope.uncheckAll = function() {
            $scope.selectedItems = [];
        };

HTML:
<div ng-repeat="item in data">
<input type="checkbox" checklist-model="selectedItems" checklist-value="item.id"> {{ item.name}}
 </div>
<button ng-click="checkAll()">check all</button>
<button ng-click="uncheckAll()">uncheck all</button>

Same issue too. I've had to find a workaround

Same issue. I fixed mine by replacing
 $scope.selection = angular.copy($scope.divisions);
with

 angular.copy($scope.divisions, $scope.selection);
(Similarily with the uncheck() function, I had to replace it with angular.copy([ ], selection))
How I came to this: I put a breakpoint in line 73 of checklist-model.js - this is meant to watch the original model change:

    // watch original model change
    scope.$parent.$watch(attrs.checklistModel, function(newArr, oldArr) {
      scope.checked = contains(newArr, value);
    }, true);

And this event never fired when I used bar = angular.copy(foo). Apparently angular was still watching the old reference to my array, but using angular.copy(foo, bar) copies the values properly.

+1

@kadikraman thank you very match :wink:

@kadikraman really well done, thank you so much. @vitalets you might want to update your code to reflect this correction, or let @kadikraman pull and add it.

+1

+1

+1 Thank you so much.

@kadikraman do you have a pull request for it? Also a test example to be added to the examples will be good.

I fixed the tests.

+1 @kadikraman code works well.
But the documentation is still wrong.

@FrancoisMartin, what exactly is wrong in the documentation so I can fix it?

Check All / Uncheck All only works with angular.copy($scope.allMyList, $scope.selectedList)
I'm using Angular 1.4.7

Does the demo not work for you? The code that you see is the code used in the demo and for me it works.

It doesn't work with Angular 1.4.7, the demo is using an older version of angular.

I updated to Angular 1.4.8. @FrancoisMartin, does it work for you?

Yes, it works fine.
My problem might be something related to my project only, so i guess we can forget this issue :)


HDDS-11394. ozone admin pipeline close --all doesn't work without parameters

What changes were proposed in this pull request?
Command ozone admin pipeline close --all close 0 pipelines. Should close all.
What is the link to the Apache JIRA
HDDS-11394
How was this patch tested?
Tested by unit tests

@Montura Could you help take a look?

Thank you @juncevich for this change.


Solve cyclic infinite bug on mismatch between backend_id and address

Steps to reproduce:

Start backend at a certain :
Create an object in it
Stop the backend (or restart it but:)
Start a new backend, with different UUID, on the same :
Try to run an active method in the old object

This should fail (the object is no longer reachable). But now, if this is done without restarting the client, ends up in an infinite cycle at the backend side (which receives a RPC that is not for themselves, but is unable to escape a retry loop and gets stuck spewing logs of
quickstart-backend-1           | WARNING:dataclay.backend.api:(f0d91203-c5cb-4757-881c-b1c8ec0c71f4) Wrong backend. Update
to 1512a0ab-6847-4577-a752-e6e3d25cc971
quickstart-backend-1           | DEBUG:dataclay.backend.api:(f0d91203-c5cb-4757-881c-b1c8ec0c71f4) Calling remote method ge
t_payroll

at eternum

An option would be to change #19 to make it fail instead of logging an error message.

One thing brought to another and #43 does fail instead of logging. That should be good enough; if it breaks other stuff we will revisit the strategy.


Private fields in class declaration expose internal identifier in declarations

🔎 Search Terms
class expressions private fields declaration
🕗 Version & Regression Information

This is the behavior in every version I tried

⏯ Playground Link
Playground Link
💻 Code
const x = class {
    #id = 10 // "__#3455@#id": number; in declarations
}

🙁 Actual behavior
The class expression is rendered in declarations as declare const x: { new (): { "__#34@#id": number; }; } exposing the internal identifier of the private field.
🙂 Expected behavior
Since we can't emit #private in an object type (like for class declarations) and we don't have class types, it's hard to come up with a solution that preserves the nominal nature of the type. I see 3 possible solutions:

Remove the type from the field (at least the internally used type will not end up being exposed) declare const x: { new (): { "__#34@#id": unknown ; }; } (partial fix)
Remove the private fields and lose the nominal nature of the class
Remove the private fields and add a unique symbol that ensures the class behaves nominally (mostly)

declare const x_private: unique symbol;
declare const x: {
    new (): {
        [x_private]: "#private"
    };
};

Third option would be the ideal one IMO. There is precedent for adding 'fake' declarations in declaration emit (_default for default exports, Name_Base for  classes that use expressions in the heritage clause)
Additional information about the issue
The numeric id is not stable, and can change based on what other private fields are seen before the one emitted in the declaration
class C { #x = 1 } // Comment out this line id is different
export  const x = class {
    #id = 10 // "__#3455@#id": number; in declarations
}

Playground Link
(It is even worse in a language service, as the id changes each time you make a change anywhere in the file)

Duplicate of #36548. Used search terms: private emit in:title


Finish switch to kramdown

This is a pr for #990.
The diff's pretty big, but almost all the new files are just me storing the highlight.js source in our repo, in case we need to rebuild from it. We're intentionally not using the latest highlight.js version, and the language auto detect will only work right if our build contains only a small number of languages, and I modified one of the language files, so I wanted to preserve all that in our repo.
For the background behind the highlight.js stuff, and the general fix being used here to our syntax highlighting woes, see this commit's message.

Nice!

Ok, hearing no objections, I'm going to merge this.
Note: a different approach here would have been to remove highlight.js entirely and set our highlighting styles solely based on the rouge-outputted server side syntax classes. That would obviously be simpler. There are a few cases, though, where the highlighting that approach produces isn't quite as good. The biggest is in the Links section on the specification page, where we have some invalid json examples that trips up rouge, but that we're able to hack highlight.js into handling correctly. The other case is that Rouge doesn't apply a distinct class to the http status in the response line, so we can't call it out from the "HTTP/1.1" part. If we decide in the future that those are sacrifices worth making, we can ditch highlight.js then pretty easily. For now, though, I just wanted to reproduce the highlighting exactly as it was.


Live trade scaffolding

Created the supporting classes of Live Trading: DataPortalLive, LiveAlgorithmExecutor & LiveTradingAlgorithm.
It is possible to start a sample application now:
(venv-python27) ~/w/s/zipline ❯❯❯ time zipline run -f zipline/examples/buy_and_hold.py --live-trading --data-frequency minute --tws-uri localhost:7500:1234     ⏎ feature/live_scaffolding ✭ ✱ ◼
[2017-05-19 14:00:55.778470] INFO: IB Broker: Connecting: localhost:7500:1234
Server Version: 76
TWS Time at connection:20170519 10:00:55 EST
[2017-05-19 14:00:56.112135] INFO: IB Broker: Managed accounts: ['xxxxxxxx']
[2017-05-19 14:00:56.216127] INFO: IB Broker: Local-Broker Time Skew: 0 days 00:00:01
[2017-05-19 14:00:56.953771] WARNING: Loader: Refusing to download new benchmark data because a download succeeded at 2017-05-19 13:37:25+00:00.
[2017-05-19 14:00:57.227565] INFO: Live Trading: initialization done
[2017-05-19 14:00:57.635889] INFO: algo: buy_and_hold.initialize()
[2017-05-19 14:00:57.639061] INFO: algo: data(): <zipline._protocol.BarData object at 0x109206c30>
[2017-05-19 14:00:59.646000] INFO: algo: data(): <zipline._protocol.BarData object at 0x109206c30>
[2017-05-19 14:01:59.913289] INFO: algo: data(): <zipline._protocol.BarData object at 0x109206c30>

I'll look into the test failures soon.

I think the failures are coming from the Yahoo! issue.


Hover over popup for helpers is not located properly in helper list

If no scrolling is done, the popup appears correctly, once we scroll, position is off and leading to a flickering browser behaviour.

@diegoduarte @Phillipoertel  mentioned this requires taking the scroll offset into account. Maybe ge can give you a pointer as how this could be fixed.

Displaying the user preview info is done with pure CSS, no JavaScript involved; see: https://github.com/lale-help/lale-help/blame/ec632ed37d59633011f112fdab9a0e072a5a729f/app/assets/stylesheets/components/_user_icon.scss#L45-L50
So subtracting jQuery's scrollTop (https://api.jquery.com/scrollTop/) to fix the position isn't possible. There's probably a solution with pure CSS as well, but you'll have to Google.


Switch away from pygithub

Because license.

I recommend checking out gidgethub. If you'd like I can look into converting one of the scripts into gidgethub? Assuming we can use Python 3.6+ here?

Sure, please go ahead. I very much used to the ways of pygithub, but if we switch I don't have preferences either way.
Also, we might have already assumed python 3.7 in some of the scripts?

I attempted to convert stuff to github3 in #79 but the PR is stale by now, so I, too, welcome a fresh approach.


updates to Vaccination Frontend

selectively disable UI for demographics at the county level when vaccination selected

minor wording / spelling / layout updates. possibly adding second large heading for definitions ?

Looks great, thanks Ben


Rip fieldy mcpartnerson

Resolves #
<Brief description of the added features or other changes/updates (either bulletpointed list, small paragraph, etc.)>

Long fp names overflows the tab component


Orthogonal indexing and MemoryCachedArray

While working on https://github.com/pydata/xarray/pull/1260 I came upon this which looks like a bug in caching:
import numpy as np
import xarray as xr
from xarray.core import indexing

nx, ny = 8, 10
data = np.arange(nx*ny).reshape(ny, nx)
cached = indexing.MemoryCachedArray(data)

data = xr.DataArray(data=data, dims=('y', 'x'))
cached = xr.DataArray(data=cached, dims=('y', 'x'))

a = data.isel(x=[2, 4], y=[3, 5])
b = cached.isel(x=[2, 4], y=[3, 5])

The last line raises:
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
<ipython-input-13-45cd1493cf6b> in <module>()
     11
     12 a = data.isel(x=[2, 4], y=[3, 5])
---> 13 b = cached.isel(x=[2, 4], y=[3, 5])

/home/mowglie/Documents/git/xarray/xarray/core/dataarray.py in isel(self, drop, **indexers)
    668         DataArray.sel
    669         """
--> 670         ds = self._to_temp_dataset().isel(drop=drop, **indexers)
    671         return self._from_temp_dataset(ds)
    672

/home/mowglie/Documents/git/xarray/xarray/core/dataset.py in isel(self, drop, **indexers)
   1141         for name, var in iteritems(self._variables):
   1142             var_indexers = dict((k, v) for k, v in indexers if k in var.dims)
-> 1143             new_var = var.isel(**var_indexers)
   1144             if not (drop and name in var_indexers):
   1145                 variables[name] = new_var

/home/mowglie/Documents/git/xarray/xarray/core/variable.py in isel(self, **indexers)
    547             if dim in indexers:
    548                 key[i] = indexers[dim]
--> 549         return self[tuple(key)]
    550
    551     def squeeze(self, dim=None):

/home/mowglie/Documents/git/xarray/xarray/core/variable.py in __getitem__(self, key)
    380         # orthogonal indexing should ensure the dimensionality is consistent
    381         if hasattr(values, 'ndim'):
--> 382             assert values.ndim == len(dims), (values.ndim, len(dims))
    383         else:
    384             assert len(dims) == 0, len(dims)

AssertionError: (1, 2)

I think I have a fix for this on my laptop -- I'll push it to GitHub tonight or tomorrow

I thought this would be fixed https://github.com/pydata/xarray/pull/1433, but this is actually a different issue.
We need to wrap the indexing result here for orthogonal indexing, but that probably shouldn't happen in the MemoryCachedArray layer. For SciPy, this happens inside ScipyArrayWrapper.__getitem__. For NumPy, this currently happens inside indexing.orthogonally_indexable.
Right now, xarray assumes that unknown array type that makes it into Variable._data supports orthogonal indexing. This is probably a mistake: we really should be more explicit and make fewer assumptions, perhaps by explicitly listing allowed array types or adding a registration system.

I noticed that PR #1639 does not fully support indexing for MemoryCachedArray.
I don't think I understand how MemoryCachedArray is being used in xarray.
What object would be stored in this array wrapper?

I don't think I understand how MemoryCachedArray is being used in xarray.
What object would be stored in this array wrapper?

When not using dask and cache=True in open_dataset, we use it to wrap arrays loaded from netCDF files.
Currently, it looks like the object loaded into it is usually a CopyOnWriteArray, which we use to ensure that modifying an xarray object never modifies the file on disk:
In [24]: ds2 = xr.open_dataset('foo.nc', engine='netcdf4')

In [25]: ds2.x.variable._data
Out[25]: MemoryCachedArray(array=CopyOnWriteArray(array=LazilyIndexedArray(array=<xarray.backends.netCDF4_.NetCDF4ArrayWrapper object at 0x11b3f25c0>, key=OuterIndexer(slice(None, None, None),))))

Thanks, @shoyer.
These array wrappers should not store a bare np.array but current code is error-prone.
I will send a fix.


Refactors profile screen component naming and fixes types missing

Date: 25-10-2024

Developer Name: Ayush Sanj

Issue Ticket Number

Closes #460
Description

Refactors profile component screen.
Fix types
remove commented code.

Documentation Updated?

[ ] Yes
[x] No

Under Feature Flag

[ ] Yes
[x] No

Database Changes

[ ] Yes
[x] No

Breaking Changes

[ ] Yes
[x] No

Development Tested?

[ ] Yes
[x] No

Screenshots

Screenshot 1

Test Coverage

Screenshot 1

Additional Notes
screen recording
https://github.com/user-attachments/assets/b6f9cc94-9ac0-49b8-a4be-424694c08b3f

@ashifkhn Added screen recording

Some minor fixes

fixed and replied accordingly


buy flow

Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.
👇 __ Let's make a quick check before the contribution.
PR Type

[ ] Bugfix
[x] Feature
[ ] Refactoring

Needs QA check

@kodadot/qa-guild please review

Testing
3 relevant places:
shopping cart, Gallery Item page, and explore
clicking buy in these places while not connected should

open connect wallet modal
after connection wallet => close wallet modal and open complete purchase moda;

clicking buy while wallet is connected => just open confirmation modal
@exezbcz
showing the loader spinner on the button and keeping the button visible while the wallet modal is open turned out quite challenging and is not included in this PR.
Context

[x] Closes #6372
[ ] Requires deployment <snek/rubick/worker>

Did your issue had any of the "$" label on it?

[x] My DOT address: Payout

Screenshot 📸

[ ] My fix has changed UI

Copilot Summary

🤖 Generated by Copilot at e3c875b
This pull request enhances the user experience of buying items from the gallery by adding a custom connect event to the ConnectWalletModal.vue component and using it to open the CompletePurchaseModal.vue component after connecting the wallet. It also refactors the openConnectWalletModal function in the useConnectWallet.ts file to accept an optional object with a callback function and a flag to control the modal closing behavior. It updates the ShoppingCartModal.vue and GalleryItemBuy.vue components to use the modified function and pass the appropriate parameters.

🤖 Generated by Copilot at e3c875b

openConnectWallet
closes after connection
autumn leaves fall fast

looks like its not absolute position or?
steps: go to explore, click buy now, connect wallet, then this appears - its closed after like 3 sec, but I could not see the confirmation dialog anymore

@exezbcz  unrelated to this PR. it happens on kodadot.xyz as well
when the shopping cart Pr was merged it worked. something broke since then

something broke since then

Hello! maybe it's me from #6575 could you try with #6591 patch?

something broke since then

Hello! maybe it's me from #6575 could you try with #6591 patch?

yes I suspect the same.
on my local shopping cart branch it works well

could you try with #6591 patch?

how?

how?

apply same change, or merge it on your PR

what is this witchcraft?
it does solve the problem
how is scoped on neoButton style cause this?

how is scoped on neoButton style cause this?

So, is it ready for testing sir?
I got this when I opened the link

also, I see some corner radius 👀 not yet on app, please

So, is it ready for testing sir?

yes

I got this when I opened the link

that's because you didn't close the modal when it was buggy, and the app state is persistent

also, I see some corner radius eyes not yet on app, please

I'll make another PR for that. don't want to mix it here

It would be better if we could reuse this part by a composable.

Thanks @Jarsen136
Done

Testing
3 relevant places:
shopping cart, Gallery Item page, and explore
clicking buy in these places while not connected should
open connect wallet modal
after a successful wallet connection => close the wallet modal and open the complete purchase modal
clicking buy while wallet is connected => just open confirmation modal

they all work, thanks for the guidance

pay 50 usd

😍 Perfect, I’ve sent the payout
💵 $50 @ 5.05 USD/DOT ~ 9.901 $DOT
🧗 1cAsKZYNRb8dkSCpn4eVkEn6ycNZTGoDo5dGDgB8J1UUWK8
🔗 0x986c5f8bca8215babb6855ee26a0d9d58445991d2c7300e50abe666166f86f34
🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues