Feature LendingLimit(KEP-1224)

What type of PR is this?
/kind feature
/kind api-change

What this PR does / why we need it:
Feature LendingLimit provides a guaranteed resource quota for user.
Which issue(s) this PR fixes:

#1224
KEP-1224
Special notes for your reviewer:
Does this PR introduce a user-facing change?

Support reserving guaranteed quota when fair-sharing among clusterQueues

/cc @kerthcet
/cc @tenzen-y

/ok-to-test

Just to confirm, here I have 2 cqs and 2 rfs.
In the first case, set  whenCanBorrow to Borrow(default)
cq-a.yaml:
apiVersion: kueue.x-k8s.io/v1beta1
kind: ClusterQueue
metadata:
  name: "cq-a"
spec:
  namespaceSelector: {} # match all.
  cohort: "cohort-ab"
  resourceGroups:
  - coveredResources: ["cpu"]
    flavors:
    - name: "arm"
      resources:
      - name: "cpu"
        nominalQuota: 500m
    - name: "x86"
      resources:
      - name: "cpu"
        nominalQuota: 600m
  flavorFungibility:
    whenCanBorrow: Borrow

cq-b.yaml:
apiVersion: kueue.x-k8s.io/v1beta1
kind: ClusterQueue
metadata:
  name: "cq-b"
spec:
  namespaceSelector: {} # match all.
  cohort: "cohort-ab"
  resourceGroups:
  - coveredResources: ["cpu"]
    flavors:
    - name: "arm"
      resources:
      - name: "cpu"
        nominalQuota: 300m
        lendingLimit: 100m
    - name: "x86"
      resources:
      - name: "cpu"
        nominalQuota: 200m
        lendingLimit: 0m

Then a workload with 600m cpu request to lq-a(belongs to cq-a) with the flavor arm(or x86) should be admitted and assigned to the flavor it requested, but 700m shouldn't, right?
And in the second case, set  whenCanBorrow to TryNextFlavor.
A workload with 600m cpu request to lq-a with the flavor arm should be admitted and assigned to the flavor x86.
In conclusion, I just want to make sure that: Borrow(or Lend) can only happen in the same flavor(like, horizontal?), right?
If there's only 1 cq and 2 rf:
  - coveredResources: ["cpu"]
    flavors:
    - name: "arm"
      resources:
      - name: "cpu"
        nominalQuota: 500m
    - name: "x86"
      resources:
      - name: "cpu"
        nominalQuota: 600m
        lendingLimit: 100m

Then Borrow(or Lend) should not work in different flavor(like, vertical?), right?
If that so, I'm going to modify KEP-1224 in Integration tests like this:

In a cohort with 2 ClusterQueues cq-a, cq-b and 2 ResourceFlavors rf-a, rf-b:

In cq-b, when rf-a's LendingLimit set, and cq-a's FlavorFungibility set to whenCanBorrow: Borrow:

In cq-a, when rf-a's BorrowingLimit unset, cq-a can borrow as much as rf-a's LendingLimit.
In cq-a, when rf-a's BorrowingLimit set, cq-a can borrow as much as min(rf-a's LendingLimit, rf-a's BorrowingLimit).

In cq-b, when rf-b's LendingLimit set, and cq-a's FlavorFungibility set to whenCanBorrow: TryNextFlavor:

In cq-a, when rf-b's BorrowingLimit unset, cq-a can borrow as much as rf-b's LendingLimit.
In cq-a, when rf-b's BorrowingLimit set, cq-a can borrow as much as min(rf-b's LendingLimit, rf-b's BorrowingLimit).

In conclusion, I just want to make sure that: Borrow(or Lend) can only happen in the same flavor(like, horizontal?), right?
Borrow(or Lend) should not work in different flavor(like, vertical?), right?

@tenzen-y Hi! Sorry to bother, but am I understanding correctly?

Then a workload with 600m cpu request to lq-a(belongs to cq-a) with the flavor arm(or x86) should be admitted and assigned to the flavor it requested, but 700m shouldn't, right?

@B1F030 If there are no admitted workloads, the workload should be assigned to arm flavor since the flavors are evaluated in order. Also, as you say, workloads with 700m CPU requests wouldn't admitted by any clusterQueue.

And in the second case, set whenCanBorrow to TryNextFlavor.
A workload with 600m cpu request to lq-a with the flavor arm should be admitted and assigned to the flavor x86.

If the workload doesn't have a nodeSelector for the arm flavor, it's correct.

In conclusion, I just want to make sure that: Borrow(or Lend) can only happen in the same flavor(like, horizontal?), right?

Yes, the Borrow will affect only the same flavors of each resource, like CPU. A workload can have only a single flavor of each resource. In other words, a workload can not have multiple flavors against a single resource, although a workload can have multiple flavors with multiple resources.
It means cpu=flavor-a and memory=flavor-b will happen, although cpu=flavor-a and flavor-b won't happen.

Yes, the Borrow will affect only the same flavors of each resource, like CPU. A workload can have only a single flavor of each resource. In other words, a workload can not have multiple flavors against a single resource, although a workload can have multiple flavors with multiple resources.

Thanks a lot!

Now the feature is basically finished, feel free to review.
I will add some tests next.

pkg/cache/: 2023-12-6 - 89.0%
pkg/metrics/: 2023-12-6 - 55.6%
pkg/scheduler/: 2023-12-6 - 81.2%

/assign

I'll pass the first review to @kerthcet.

/retest

/retest

/retest

/retest

/release-note-edit
The lendingLimit field in ClusterQueue's quotas allows restricting home much of the unused resources by the ClusterQueue can be borrowed by other ClusterQueues in the cohort. In other words, this allows a quota equal to `nominal-lendingLimit` to be exclusively used by the ClusterQueue.

/retest

I'll wait for an LGTM from @kerthcet before giving another pass.

/retest

/retest

/retest

/retest

/retest

/retest pull-kueue-test-e2e-main-1-29

/retest

@tenzen-y how do you feel about merging this PR this week?

/lgtm
/approve

/release-note-edit
Add a lendingLimit field in ClusterQueue's quotas, to allow restricting how much of the unused resources by the ClusterQueue can be borrowed by other ClusterQueues in the cohort.
In other words, this allows a quota equal to `nominal-lendingLimit` to be exclusively used by the ClusterQueue.


feat(BidirectionalTool): Update tool data on every modified event of …

Issue

Once tool handlers are modified we trigger a MEASUREMENT_MODIFIED event but it does not get toolData updated. So other components that listen to it does not get properly updated.

Fix

Wait image render and submit the modified event with the toolData updated.
Also moved the tool data calculation into another file/function.

It looks like CI is failing. Can you sync it with the base branch and resolve any issues? Feel free to ping me if the mistake/hold-up is on my end.

@dannyrb Just rebased my branch.
There is an issue with the tests, because our docker image uses latest version of NodeJs (11.11.0) and its causing issues with Jest.

Do the tests pass when you change:
https://github.com/cornerstonejs/cornerstoneTools/blob/e90a7ad405960525dc46de3e4a2048655091a2d8/.circleci/config.yml#L6
To: - image: circleci/node:10.0.0

Hmm.. Apparently not. My next best guess would be a cache issue with packages then? You could try commenting out the save/restore cache portions of the CI config?
I'll look into this issue more next week if you don't have the time right now.

Thanks, @galelis!

:tada: This PR is included in version 3.4.0 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


[Event Request] page 6510 "Item Tracking Lines" - OnRegisterChangeOnAfterInsert

We would like to add a new event OnRegisterChangeOnAfterInsert in the following procedure:
local procedure RegisterChange(var OldTrackingSpecification: Record "Tracking Specification"; var NewTrackingSpecification: Record "Tracking Specification"; ChangeType: Option Insert,Modify,FullDelete,PartDelete,ModifyAll; ModifySharedFields: Boolean) OK: Boolean

.
.
.
                if Item."Order Tracking Policy" = Item."Order Tracking Policy"::"Tracking & Action Msg." then
                    ReservEngineMgt.UpdateActionMessages(ReservEntry1);

                if ModifySharedFields then begin
                    ReservEntry1.SetPointerFilter();
                    ReservEntry1.SetTrackingFilterFromReservEntry(ReservEntry1);
                    ReservEntry1.SetFilter("Entry No.", '<>%1', ReservEntry1."Entry No.");
                    ModifyFieldsWithinFilter(ReservEntry1, NewTrackingSpecification);
                end;

                OnRegisterChangeOnAfterInsert(NewTrackingSpecification, OldTrackingSpecification, CurrentPageIsOpen); //A200F0000.Event.n
                OK := true;
            end;

Event Signature:
[IntegrationEvent(false, false)]
local procedure OnRegisterChangeOnAfterInsert(var NewTrackingSpecification: Record "Tracking Specification"; var OldTrackingSpecification: Record "Tracking Specification"; CurrentPageIsOpen: Boolean)
begin
    //A200F0000.Event
end;

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one.


Hyprpicker for gentoo's portage?

I really want to use hyprpicker but it is not in the gentoo repos.
Is anyone thinking of adding it?
If no one is I might try learning how to do so.

it's not our job to maintain packages


Typo correction: See slso >> See also

1 letter typo correction.
Reported in issue ticket #5024 (See slso => See also).
Thanks to @mehmetberber for finding and reporting this.
#sign-off
Closes #5024

@illfated : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@illfated, you can only sign off in the main PR conversation, not in review comments.

@illfated: I'm sorry - only the author of this article, @JasonGerend, can sign off on your changes. But we do have an exception process - if you are on the Microsoft content or product team for this product area, you can ask the PR review team to review and merge it by sending mail to the techdocprs alias.


Support NEGs in net-glb backends

The healthcheck configuration prevents using NEGs in the module backends:
Error: Error creating BackendService: googleapi: Error 400: Invalid value for field 'resource.healthChecks': ''. Backend services with global network endpoint groups can have health checks only for INTERNAL_SELF_MANAGED load balancing scheme and INTERNET_FQDN_PORT endpoint type., invalid

Hi @ludoo thanks for flagging this.
I tried to reproduce the bug with the following code, but I wasn't able to reproduce it.
I'll close the bug for now, but please let me know if issues arise again.
module "glb" {
  source     = "../cloud-foundation-fabric/modules/net-glb"
  name       = "glb-test"
  project_id = "xxx"

  # This is important as serverless backends require no HCs
  health_checks_config_defaults = null

  backend_services_config = {
    my-serverless-backend = {
      bucket_config = null
      enable_cdn    = false
      cdn_config    = null
      group_config = {
        backends = [
          {
            group   = google_compute_global_network_endpoint_group.neg.id
            options = null
          }
        ],
        health_checks = []
        log_config = null
        options = null
      }
    }
  }
}

resource "google_compute_global_network_endpoint_group" "neg" {
  name                  = "internet-neg"
  default_port          = "80"
  network_endpoint_type = "INTERNET_FQDN_PORT"
  project               = "xxx"
}

resource "google_compute_global_network_endpoint" "default-endpoint" {
  global_network_endpoint_group = google_compute_global_network_endpoint_group.neg.name
  fqdn       = "www.google.com"
  port       = 80
  project    = "xxx"
}


Feture request: --silent flag

It would be great if there was a silent flag available

What would that do that's different from > /dev/null?

@ljharb not sure if this is enough of a reason, but enable similar behavior to > /dev/null on Windows?

@webketje ah, that's a fair point - does the windows cmd shell not have a way to redirect output?


Inquiry Regarding Data and Implementation for Reproducing Results from Study

The dataset shared appears incomplete, and some experimental training data is missing. Could you kindly provide the detailed datasets for the five experiments mentioned in the paper?

If you wish to request additional data beyond the materials disclosed in this repository, please contact the corresponding author, specifying your affiliation and the intended purpose of use.


Can't display the blank lines in code block correctly

I'm using hexo-theme-vexo released at 2017-09-21.
When I applied this theme for my blog site, I found that when render code blocks, it couldn't display the blank lines correctly. Also, the syntax highlighting of bash program language was incorrect.

fixed in #42

you can download the latest release https://github.com/yanm1ng/hexo-theme-vexo/releases/tag/v2.2


Update pg_materialized_view.py

I had problems using this to create materialized views with indizes cause of alembic migration tried to replace the views without indizes every time a new autogenerate has to be used. We deactivated dropping of type: 'materialized views' in alembic.ini and build it with replaceableObject of alembic migration, but this changes at pg_materialized_view.py resolve the whole problem
Materalized view would be created like this then, multicolumn-index is possible also, same as statement without indizes-parameter:
test_persons_matview = PGMaterializedView(
schema='public',
signature='test_persons',
definition="""select firstname, lastname, city from public.persons
""",
with_data=False,
indizes=[{'name': 'test_persons_lastname_idx', 'columns': 'lastname'},
{'name': 'test_persons_city_idx', 'columns': 'city'}]
)

Thank you for opening a PR
Supporting indexes on materialized views is a feature I'm interested in but this PR would need changes before it could be accepted

The type of indexes would need to be List[Index] from sqlalchemy.Index and support unique, non-unique, single and multi-column
cls.from_database does not currently pull the indexes down from the database. That would cause any materialized view with an index to produce an op.replace_entity any time --autogenerate was used
cls.get_required_migration_op would also have to change because it would not detect difference in indexes
test cases

There are some other complications related to drop cascades and dependency resolution but if you have the interest to tackle the list above, I'm happy to handle the resolver/weird stuff since its pretty hairy

@olirice Turns out that this is a feature I'm going to need myself now. In terms of the API, you're happy with extending PGMaterializedView to take a list of sqla indexes? If so I'll start another PR taking a run at this myself.

@olirice this is a bit of a curious one. Pulling down the indexes is simple enough (sqla even supports doing it directly using their reflection api), but all the sqla tooling is around having a proper table to attach the index to.
I was looking into the possibility of making a fake table to make sql happy and then we could maybe piggy-back on the existing alembic index migration code. Again, I think you could probably make that work, but on closer inspection it's not going to work for me anyway because sqla won't reflect the more complex indexes I need to support.
Certainly in my case, it would be simpler to compare pg_indexes.indexdef against my own create statement string and I'd be able to tell what needing creating / dropping.
Looking at the sqla / alembic code it almost feels like it's much harder there because you have to support generalised reflection of arbitrary index definitions (eg CREATE INDEX complex_trgm_idx ON blah USING gin (lower(text_field) gin_trgm_ops)). There's a lot more parsing and work to do to create these (which isn't supported currently).

@aidos thank you for the summary of what you looked into
I agree that this is a big undertaking. I've been maintaining materialized view indexes manually in migrations rather than developing objects for them for that reason

I've been deeper into the alembic / sqlalchemy side now and I'm going to start from that direction since I actually need to support more complex indexes than they're currently handling. I'll get it so that expression based indexes (eg lower(name) etc) are working first. Once that's done I'll revisit the materialised views aspect to see if there's a reasonable workaround.

closing for age


PlatformException when call method channel

Describe the bug
Getting platform exception after call AdyenCheckout.instance.getReturnUrl()
PlatformException(channel-error, Unable to establish connection on channel: "dev.flutter.pigeon.adyen_checkout.CheckoutPlatformInterface.getReturnUrl"., null, null)
To Reproduce
Steps to reproduce the behavior:

Install using: flutter pub add adyen_checkout
Add AdyenCheckout.instance.getReturnUrl() to a FutureBuilder() widget and print the snapshot.error to see the error.

Expected behavior
Expected an String with return url.
Screenshots

Smartphone (please complete the following information):

Device: Pixel3a (emulator)
OS: api 34

Additional context
I'm using Flutter 3.19.0 and Dart 3.3.0.
Here is a snippet: https://gist.github.com/costaaaaaa/c1631f705509089bc945fc083b563522

Same issue

Found what happening: https://github.com/Adyen/adyen-flutter/issues/292 solved my problem.
Just use FlutterFragmentActivity on android/MainActivity.kt

It worked! thank you.
Eventho I don't get what's the difference.

With a little search I founded this:
FlutterActivity: Your app will be closed.
FlutterFragmentActivity : Your app will be move to background.


adds basemaps from Carto

Adds base maps from:
https://carto.com/help/building-maps/basemap-list/#carto-vector-basemaps
Attribution modeled after documentation at:
https://carto.com/help/working-with-data/attribution/#basemaps

Since we switch to semantic-release, can you prefix the name of the PR with perf:  -- this will make a point release when the branch is merged,
One of our examples shows off the list of defined tilesets (see examples/measure/index.pug).  Can you add these to it?
Perhaps as a separate PR, we could add preferred human-readable labels to these definitions and then populate the example from the dictionary of values.

I can certainly add these to the example and make those changes.

I added name to the tileSources definition and changed the example to load the tileSources dynamically. Let me know what you think.


JavaScript: Improve handling of destructuring assignments.

We previously modelled them as parallel assignments to a number of variables, but in fact these assignments happen in sequence, which can sometimes lead to false positives. While fixing this I noticed that our CFG for destructuring assignments is also wrong: where for a normal assignment lhs = rhs we first evaluate lhs to a reference and then evaluate rhs, in a destructuring assignment rhs is evaluated first. This is fixed in the corresponding internal PR.
Profiling then showed a number of performance regressions in the SSA library, which are fixed by the other commits. Per-commit review strongly encouraged.

We need more change notes for this than the one in the related internal PR, don't we?

What would you like me to add?

Internal PR has been merged, so merging this one as well.


Add "Join our Slack" button on the front page next to "Get Started"

We have gotten some community feedback regarding it's hard to find the Slack link on our front page. It's currently just a small icon on the top right corner. It's been suggested we add a "Join our Slack" button on the front page next to "Get Started".
What do you think @qiujian16 ?

yes, I think it sounds good

/assign mikeshng
I am not an UI expert but I will give it a try first.

Done.


And doc in wiki on how to configure ssl in nginx

It's very important in a tr069 server and other software in general to use ssl encryption

fantastic @penaivanalejandro. ill add it to wiki soon :)

Thanks @penaivanalejandro. added to wiki on https://github.com/freeacs/freeacs/wiki/Setup-SSL-on-NGINX. Closing issue


Return type from pandas.core.groupby.generic.DataFrameGroupBy.resample

Describe the bug
The type hint for pandas.core.groupby.generic.DataFrameGroupBy.resample says it returns a pandas.core.groupby.grouper.Grouper; however in at least some situations this function can return a pandas.core.resample.DatetimeIndexResamplerGroupby, which is not a Grouper.
To Reproduce
This results in a false positive on MyPy 1.7.1:
df.set_index("date").groupby("intensity").resample("90D")["duration"]

The error message is
error: Value of type "Grouper" is not indexable  [index]

Please complete the following information:
Python 3.11
Mypy 1.7.1
pandas-stubs <IP_ADDRESS>928

Thanks for the report.  Looking at the docs, we should change
to return a union of pandas.api.typing.DatetimeIndexResamplerGroupby,
pandas.api.typing.PeriodIndexResamplerGroupby, and
pandas.api.typing.TimedeltaIndexResamplerGroupby
Unfortunately, we haven't added the types for pandas.api.typing yet, nor do we have DatetimeIndexResamplerGroupby in core/resample.pyi .  So that latter type should get added there,  and can be the return type of resample(), since the other 2 types are subclasses of the first one.
Some care will need to be exercised, because in the stubs we are doing some things with respect to generic types that will have to be appropriately passed down for things to work.


Fix readme

Coverage remained the same at 97.727% when pulling 2f13dbeba8a897c6fdbb3092ccaa7bd287f64e0f on icecreamhead:patch-1 into b8cc0c60b8d804cfd7f3369689576b9a93ea1e6f on kcthota:master.


32 libraries

Some scripts we use assume a 32bit OS & rely on older libraries. Previously installing ia32libs via apt get would sort it out, seems this is replaced with  lib32z1.  I used on this the vm & it seems to work - should this be in the playbook?

If these script are part of the VM then 32bit compatibility libs should be installed. If these scripts are not part of the VM then the script installation documentation should explain how to get it running on a 64bit machine.
Which scripts are you talking about?


ign -> gz Migrate Ignition Headers : gz-gui

See: https://github.com/gazebo-tooling/release-tools/issues/784

Some of these suggestions may be ignored depending on your plan/workflow for renaming but the code comments in the migrated headers should be updated from ign* to gz/gazebo

Thanks for the reviews!
I think the docs/comments migration will be happening in a separate (easier to merge) wave, since they can really make reviewing some of the larger libraries difficult (e.g. sim), same for the github URL changes.
For the macro and CMake macro changes, I don't think the upstream changes that support them has been made, since only headers were migrated this wave.
I'll fold in the broken URL changes though, thanks for catching those! :bow:

@methylDragon what's the status of this?

@methylDragon what's the status of this?

This was supposed to be ready but pending the release of upstream libraries. I'm not sure about now though, changes might have occurred between then and now might mean new stuff added might need to be migrated.

Are there any plans to revisit this?

Are there any plans to revisit this?

Hmm, I think I won't be able to spend active time on this.. Should I close this PR?

I'll pick up this PR.


Bug on extend_start.sh

Hi,
my container octavia-api kept restarting without giving me any logs. Internally on the container i saw this error:
line 22: /var/lib/octavia: Is a directory
After i removed the tilde at the end of the file the container started successfully. Could be a bug?

Thank you for contributing to openstack/kolla!
openstack/kolla uses Gerrit for code review.
If you have never contributed to OpenStack before make sure you have read the
getting started documentation:
https://docs.openstack.org/infra/manual/developers.html#getting-started
Otherwise please visit
https://docs.openstack.org/infra/manual/developers.html#development-workflow
and follow the instructions there to upload your change to Gerrit.


Bom for thumbtroller is missing

I couldnt find the bom for the controller. Could you also add it?

It's in the Thumbtroller repository!
https://github.com/StuckAtPrototype/Thumbtroller/tree/master/hardware

closing this out. its here https://github.com/StuckAtPrototype/Thumbtroller/blob/master/hardware/controller-v0.2-BOM.csv


Revert "org.glassfish.hk2:hk2-locator 2.2.0-b21"

Tried to fix curation but version is not offered in this PR.


Add new badge for Facebook sharing

following the Twitter tweet badge pattern
the FB logo is from Wikimedia Commons: https://commons.wikimedia.org/wiki/File:F_icon.svg

Preview:

Apparently not enough interest for this.

@marcelstoer Would you like to reopen this? As you can see things are moving in this project.

As you can see things are moving in this project.

Uhhm, / can't see that as I'm no longer following this project. Care to point me into the right direction?
I resorted to placing all the static shields in the GitHub gh-pages branch and then serving them from there. Fixes quota and shields.io overload issues as well. Example: https://github.com/marcelstoer/nodemcu-pyflasher/

Here's github's summary of last month's activity. There's also a new testing framework intended to make the review process easier (among other goals).


[Webpack 5] Default export for JSON modules for arrays

Bug report
Assuming you have a JSON file prices.json with the following content:
[
    {
      "key": "startup",
      "title": "Startup",
      "price": 75
    },
    {
      "key": "basic",
      "title": "Basic",
      "price": 150,
      "featured": true
    },
    {
      "key": "partnership",
      "title": "Partnership",
      "price": null
    }
  ]

Note that the root object is simply an array.
And you import it using the default export as required by Webpack 5:
import pricing from "./pricing.json";

What is the current behavior?
Gives an error when Next.js tries to compile:
TypeError: undefined is not a function

If the current behavior is a bug, please provide the steps to reproduce.

What is the expected behavior?
I believe it should be possible to map the prices.
prices.map((price) => console.log)

Changing the prices.json to be an object with a key works:
{
  "prices": [
    {
      "key": "startup",
      "title": "Startup",
      "price": 75
    },
    {
      "key": "basic",
      "title": "Basic",
      "price": 150,
      "featured": true
    },
    {
      "key": "partnership",
      "title": "Partnership",
      "price": null
    }
  ]
}

Works now using the additional key
prices.prices.map((price) => console.log)

Other relevant information:
webpack version<EMAIL_ADDRESS>Node.js version: v12.18.3
Operating System: MacOS 10.15.7
Additional tools:

Please open an issue in Next.js repo,

Also json doesn't have default export

This looks like a bug. Do you have a stack trace for this error?
Is it reproducible without next.js?

The error suddenly manifested when I upgraded Webpack from 5.0.0-beta.28 to 5.0.0-rc.3 so I assumed it was a Webpack issue. 🙂
Full stack trace:
Error occurred prerendering page "/". Read more: https://err.sh/next.js/prerender-error
TypeError: undefined is not a function
    at PricingTable (/Users/michiel/Documents/monitr/repos/monitr.be/.next/server/pages/index.js:2983:32)
    at d (/Users/michiel/Documents/monitr/repos/monitr.be/node_modules/react-dom/cjs/react-dom-server.node.production.min.js:36:498)
    at $a (/Users/michiel/Documents/monitr/repos/monitr.be/node_modules/react-dom/cjs/react-dom-server.node.production.min.js:39:16)
    at a.b.render (/Users/michiel/Documents/monitr/repos/monitr.be/node_modules/react-dom/cjs/react-dom-server.node.production.min.js:44:476)
    at a.b.read (/Users/michiel/Documents/monitr/repos/monitr.be/node_modules/react-dom/cjs/react-dom-server.node.production.min.js:44:18)
    at renderToString (/Users/michiel/Documents/monitr/repos/monitr.be/node_modules/react-dom/cjs/react-dom-server.node.production.min.js:54:364)
    at renderPage (/Users/michiel/Documents/monitr/repos/monitr.be/node_modules/next/dist/next-server/server/render.js:50:851)
    at Object.ctx.renderPage (/Users/michiel/Documents/monitr/repos/monitr.be/.next/server/pages/_document.js:675:28)
    at Function.getInitialProps (/Users/michiel/Documents/monitr/repos/monitr.be/.next/server/pages/_document.js:153:19)
    at Function.getInitialProps (/Users/michiel/Documents/monitr/repos/monitr.be/.next/server/pages/_document.js:679:83)

I haven't been able to create a repro without Next.js though.

@MichielDeMey If you create reproducible test repo with next I can point to bug line

I've create a minimal repo here: https://github.com/MichielDeMey/nextjs-webpack5
Uses Next.js 9.5.4-canary.23, which has been updated to fix a change in the configuration schema introduced in Webpack ^5.0.0-beta.28. Webpack version used is ^5.0.0-rc.3.

@MichielDeMey thanks for repro, we will work on fix.
fast fix for you will be to change webpack config option manually via https://nextjs.org/docs/api-reference/next.config.js/custom-webpack-config
config.optimization.concatenateModules = false;

https://webpack.js.org/configuration/optimization/#optimizationconcatenatemodules

As noted in https://github.com/webpack/webpack/issues/11588, it doesn't seem to be Next.js specific (I'm using pure Webpack).
The version in which mapping over arrays imported from JSONs seem to have been broken is 5.0.0-beta.31.

@wojtekmaj It is not Next.js specific, it is bug in webpack and we are working on fix


[FEATURE]: (short description)

Hello,
I'm facing an issue when cropping an orthomosaic into tiles with solaris.tile.raster_tile.RasterTiler(). My orthomosaics covers a small area (155 m2) and when I crop it into smaller pieces, files are saved in a folder with the following name format : [src-filename]_[longitude]_[latitude].tif. However, longitude and latitude within the file name gets 3 numbers after the coma which is annoying to me because my orthomosaic doesn't cover a large area so, with 3 numbers after the coma, I only get 2 different longitude/latitude values. When saving after cropping, tiles then get the same dest name and are overwritten.
I would like to choose the accucary of longitude and latitude with an argument like dest_fname in the tile() function.
Currently, I have edited the save_tile() in raster_file.py to take 7 numbers after the coma in order to be able to get all my files saved and not overwritten:
            dest_fname = "{}_{}_{}.tif".format(
                dest_fname_root,
                np.round(profile["transform"][2], 7),
                np.round(profile["transform"][5], 7),
            )

Same comment when saving vector.


WIP: Adding 30px margin Well / FCM

WIP: Adding 30px margin Well / FCM

Screenshots

Additions
Changes

Modifeid cfgov/jinja2/v1/browse-basic/index.html to remove the block__flush-sides class.
Modified cfgov/jinja2/v1/browse-filterable/index.html to remove the block__flush-sides class.
Modifed cfgov/jinja2/v1/newsroom/index.html to remove the block__flush-sides` class.
Modifed cfgov/jinja2/v1/sublanding-page/index.htmlto remove theblock__flush-sides` class.

Testing

Run gulp styles.
Visit http://localhost:8000/about-us/the-bureau/
Visit http://localhost:8000/consumer-tools/prepaid-cards/.

Screenshots
Notes

The featured content module, which contains a video player, will need to be refactored. The share buttons get cropped at breakpoints lower than 950px:

Todos

Checklist

[ ] PR has an informative and human-readable title
[ ] Changes are limited to a single goal (no scope creep)
[ ] Code can be automatically merged (no conflicts)
[ ] Code follows the standards laid out in the front end playbook
[ ] Passes all existing automated tests
[ ] New functions include new tests
[ ] New functions are documented (with a description, list of inputs, and expected output)
[ ] Placeholder code is flagged
[ ] Visually tested in supported browsers and devices
[ ] Project documentation has been updated
[ ] Reviewers requested with the Assignee tool :arrow_right:

@sebworks I think all we really need to do is remove this kind of logic from each of the page templates that allow FCMs and Wells.

And remove the classname from the Well definition: https://github.com/cfpb/cfgov-refresh/blob/a5a3320ac4b2874b6213334cee32c0c498b6700d/cfgov/v1/atomic_elements/organisms.py#L30

@Scotchester, I agree 1000000%. It just got out of control at the end man. Wrapping things in blocks, logic in templates, class names in logic, parse_links, etc...

@Scotchester, I've updated the code to remove the CSS and just use the blocks. This honestly feels like the wrong approach though. This approach forces you to wrap every featured content module and well in a div.block. It also forces you write an if statement in every template you want to use the those components. I'm going with this for now because it's how we are currently doing things.
I'm now working on refactoring the featured content module video player.

We're already wrapping them in div.block though, and not just for FCM and Wells, but for every Wagtail module. Because of that, you should be able to actually remove the entire {% if 'featured_content' in block.block_type %} condition from those templates.

We're already wrapping them in div.block though, and not just for FCM and Wells, but for every Wagtail module. Because of that, you should be able to actually remove the entire {% if 'featured_content' in block.block_type %} condition from those templates.

Unfortunately, you can't do that because some components aren't wrapped and others use different block modifiers.

Unfortunately, you can't do that because some components aren't wrapped and others use different block modifiers.

You can, I promise :)
If the {% if 'featured_content' in block.block_type %} condition fails, the else is to just go to render_block, in which all blocks get a div.block wrapper.

@Scotchester, have you looked at: https://github.com/cfpb/cfgov-refresh/blob/master/cfgov/jinja2/v1/sublanding-page/index.html?
if you are wrapping every block in a div then just handle it with CSS. Why have the else logic in the template?

Should we close this in favor of the discussions going on in the Design Manual?

Should we close this in favor of the discussions going on in the Design Manual?

Yep, with pleasure.


[Question] Any chance bringing the forks together?

Hey there!
First, thanks for making this all happen, it helps me a lot in my daily work!
Secondly: Is there a chance, that you/we can get all forks on one table and see how this can be beneficial for the project? For example: You and also https://github.com/ngaut/rust-rocksdb seem to have made pretty good progress, but the branches are too diverged and I lack of knowledge to merge them my self.
So I try and shoutout to @ngaut, @BusyJay and @zhangjinpeng1987 to join the discussion if those two forks should be one again?
Hope that helps somehow ...
Cheers,
Mario

Yeah! ngaut's (being used for the tikv project) is the main one I would
like to pull in! Right now I'm backpacking in the Alps though and will be
away from a computer until the middle of October, but I can start to work
on this more when I get back! If you would like to work on this at all
before then just say so and I'll be able to review/merge from my phone at
some point every other day or so!
On Sep 29, 2016 07:49, "Mario Mueller"<EMAIL_ADDRESS>wrote:

Hey there!
First, thanks for making this all happen, it helps me a lot in my daily
work!
Secondly: Is there a chance, that you/we can get all forks on one table
and see how this can be beneficial for the project? For example: You and
also https://github.com/ngaut/rust-rocksdb seem to have made pretty good
progress, but the branches are too diverged and I lack of knowledge to
merge them my self.
So I try and shoutout to @ngaut https://github.com/ngaut, @BusyJay
https://github.com/BusyJay and @zhangjinpeng1987
https://github.com/zhangjinpeng1987 to join the discussion if those two
forks should be one again?
Hope that helps somehow ...
Cheers,
Mario
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/spacejam/rust-rocksdb/issues/69, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABb40J2MVp5hH2cPrBXiydB6lVEQw3wSks5qu1FRgaJpZM4KJmnu
.

Hi, @xenji We would like to merge all of the changes to the repo. It's just there is a problem:
The new interfaces are not compatible with the previous version of rust-rocksdb.
And that could bother users who are already  used rust-rocksdb in their project.

I'm happy cutting a new major number for an api breaking new version
On Sep 30, 2016 07:57, "goroutine"<EMAIL_ADDRESS>wrote:

Hi, @xenji https://github.com/xenji We would like to merge all of the
changes to the repo. It's just there is a problem:
The new interfaces are not compatible with the previous version of
rust-rocksdb.
And that could bother users who are already used rust-rocksdb in their
project.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/spacejam/rust-rocksdb/issues/69#issuecomment-250665165,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABb40A2S_mwPEWeg-xb5CYgestkcgoI_ks5qvKS5gaJpZM4KJmnu
.

Seems like a plan. Good to have you all in the conversation!
Should we revive this discussion and dive deeper when @spacejam is back from the Alps? I think this needs more than just a bit of talking, doesn't it?

@spacejam If you stick to Semantic Versioning you don't even need to cut a new major version as long as you don't have a 1.0.0 release yet.

Any updates on all this?

Hey Everyone,
I'd just like to say, I'm keen to help out with improving the general quality of this wrapper (such as writing API docs and improving the tests). I also need some more features of RocksDB for my project, which I'm happy to put some of my own time in to implementing.
It looks like some of this work may have already been done in the other forks though. I'm not sure what I should do as my availability to help out will deminish after this week but any work I do now will probably conflict or duplicate with what has been done in the other forks.
I'm happy to help out in any way I can to get the changes in the forks pushed upstream so let me know if you need an extra pair of hands.

Good job. @kaedroho
I think you may port all of the changes from my Repo which is currently used by TiKV project. We are still adding more and more features to that Repo. And i guess your porting process will break the current API.
Any thoughts? @spacejam

Thanks @ngaut
Just been having a look. It appears the fork was made a while ago and both itself and upstream have had significant changes since then. This complicates things a little bit.
I reckon our best course of action is:

Review changlog of ngaut's branch, create a list of pieces of work that have been done on that branch (me)
Agree which items on that list should be ported and priority (everyone)
For each feature, port it over to the upstream fork, add any docs/tests that may be missing, create a PR (me)
Review the PR and merge (@spacejam, @ngaut)

Does this sound OK?

SGTM. @kaedroho

Here's the list of the pull requests that have been submitted against ngaut's fork:
Code cleanup

Cleanup https://github.com/ngaut/rust-rocksdb/pull/2
Reorganise code https://github.com/ngaut/rust-rocksdb/pull/6
Tiny cleanup https://github.com/ngaut/rust-rocksdb/pull/20
Refactor options https://github.com/ngaut/rust-rocksdb/pull/38
Cleanup test directory https://github.com/ngaut/rust-rocksdb/pull/40
Make code more rusty https://github.com/ngaut/rust-rocksdb/pull/41
Change line width to 100 https://github.com/ngaut/rust-rocksdb/pull/47

Static linking

Support static linking https://github.com/ngaut/rust-rocksdb/pull/26
Detect stdc++ automatically https://github.com/ngaut/rust-rocksdb/pull/27
fix header not found https://github.com/ngaut/rust-rocksdb/pull/28
Add ffi_try! to simplify error handling https://github.com/ngaut/rust-rocksdb/pull/46
Use default c++ if specified https://github.com/ngaut/rust-rocksdb/pull/48

Iterators

Export Iterator.valid() https://github.com/ngaut/rust-rocksdb/pull/7
Make iterator not copy https://github.com/ngaut/rust-rocksdb/pull/9
Refactor iterator https://github.com/ngaut/rust-rocksdb/pull/12
Remove start key when create iterator https://github.com/ngaut/rust-rocksdb/pull/13 and https://github.com/ngaut/rust-rocksdb/pull/23
Allow setting iterate option https://github.com/ngaut/rust-rocksdb/pull/16

Options

Add compression option https://github.com/ngaut/rust-rocksdb/pull/8
Add max_bytes_for_level option https://github.com/ngaut/rust-rocksdb/pull/10
Add disable_wal option https://github.com/ngaut/rust-rocksdb/pull/11
Add report_bg_io and compression_per_level options https://github.com/ngaut/rust-rocksdb/pull/19
Add set_bloom_filter and set_cache_index_and_filter_blocks to BlockBasedOptions https://github.com/ngaut/rust-rocksdb/pull/25
Add set_level_zero_file_num_compaction_trigger option https://github.com/ngaut/rust-rocksdb/pull/30
Add set_iterator_upper_bound to ReadOptions https://github.com/ngaut/rust-rocksdb/pull/31

Not actually a RocksDB option
Methods now require ownership of ReadOptions (which prevents reuse)

Add set_max_manifest_file_size option https://github.com/ngaut/rust-rocksdb/pull/33
Add set_wal_recovery_mode option https://github.com/ngaut/rust-rocksdb/pull/34

follow up fix: https://github.com/ngaut/rust-rocksdb/pull/36

Add enable_statistics and set_stats_dump_period_sec options https://github.com/ngaut/rust-rocksdb/pull/42
Add set_compaction_filter option https://github.com/ngaut/rust-rocksdb/pull/43
Add set_num_levels option https://github.com/ngaut/rust-rocksdb/pull/45
Add get_statistics option https://github.com/ngaut/rust-rocksdb/pull/49

Miscellaneous

Add flush and approximate size support https://github.com/ngaut/rust-rocksdb/pull/3
Add get and get_cf methods to Snapshot https://github.com/ngaut/rust-rocksdb/pull/4
Add delete files in range https://github.com/ngaut/rust-rocksdb/pull/14

Note: “files” should be pluralised. The RocksDB CAPI is wrong

Add path method to DB https://github.com/ngaut/rust-rocksdb/pull/15

Q: Do DBs always have a path?

Support get_property

https://github.com/ngaut/rust-rocksdb/pull/17
https://github.com/ngaut/rust-rocksdb/pull/18

Introduce UnsafeSnap https://github.com/ngaut/rust-rocksdb/pull/21
Add count and is_empty methods to WriteBatchhttps://github.com/ngaut/rust-rocksdb/pull/22

Q: Should count be len?

update_cf https://github.com/ngaut/rust-rocksdb/pull/24
Add list_column_families API https://github.com/ngaut/rust-rocksdb/pull/37
Make compact_range arguments optional https://github.com/ngaut/rust-rocksdb/pull/44

Here's my opinions
Code cleanup
There has been some major refactoring and cleanups in the fork, but upstream has been heavily changed too. I don't think it's worth, and may not even be feasible to port these.
If we must reproduce these cleanups, we should probably get them out of the way first so avoid merge conflicts in new changes.
Static linking
I think it would be great to get this upstream. I had problems with jemalloc segfaulting under a dynamically linked RocksDB.
The ethcore fork have implemented this as well. They appear to have done it differently and also have Windows support so I think it's worth looking into both implementations.
Iterators
There's been a lot of work around refactoring the iterator API and I disagree with some of the design decisions. I think we should discuss this part of the API before making any changes.
Options
Most of these changes follow a similar pattern by just exposing new functions in the Options struct. We could port all of these in one go. I had to do some digging to find out what some of them do, so I think it would be a good idea to also add docstrings for all of them as we do the porting.
There's one change that I disagree with though, https://github.com/ngaut/rust-rocksdb/pull/31. This adds an upper-bound to range iterators but it's not part of the standard RocksDB API. The PR also changes the read methods to consume the ReadOptions object which prevents reuse.

Thanks for your excellent work. @kaedroho
About Options:
Upper-bound iterator is actually one of the standard RocksDB API. See https://github.com/facebook/rocksdb/blob/master/include/rocksdb/options.h#L1447
And it's really useful in some scenarios.  I insist that we should keep it.
Others sounds great to me.

Thank you very much for stepping forward to help with this, @kaedroho!
I can imagine the upper bound being a nice way to help with things like multiplexing higher level lexicographic ranges of sharded databases, and getting higher performance than filtering results after the fact. I'm in favor of keeping it.
In general I'm in favor of making the merge path for @ngaut's changes as seamless as possible, as he is doing a ton of great work that I'd love to merge in over time. To that end, I'm generally in favor of keeping whatever API semantics his version has (maybe rethinking ownership of ReadOption though), breaking the ones here where necessary, adding proper documentation (I can help with this over time).
I can commit a couple hours per week to this to help this effort, but it may look more like a whole day per month or something like that due to random traveling. Since you want to cram work into this week, I'll try to pay extra close attention to this to clear blockers on my end to your progress.

Upper-bound iterator is actually one of the standard RocksDB API.

Ahh, my mistake! It looks like a bit of dead code got through on that pull request. See: https://github.com/ngaut/rust-rocksdb/pull/31/files#diff-15fa9749208206c5a07bbe0cc9aa11abR45
This new attribute is the reason why the ownership rules had to change, but it doesn't look like it was ever used anywhere, so I think it can just be removed. I'll just port the set_iterate_upper_bound method across.

Thanks for your excellent work. @kaedroho
Thank you very much for stepping forward to help with this, @kaedroho!

No problem! This wrapper has been really useful to me so far, glad I can help make it better!

In general I'm in favor of making the merge path for @ngaut's changes as seamless as possible, as he is doing a ton of great work that I'd love to merge in over time.

I agree, the TiKV team have been making some really useful changes. But I think it's a shame that both TiKV and Ethcore have decided that they must maintain their own forks in isolation of each other. Both projects have implemented static linking (a huge bit of work) when only one of them really needed to and new projects in the future will not benefit from either of their efforts!
I hope this effort would be a step towards bringing all the teams together, so we can all work on the same codebase and benefit from each other's contributions.
I've just submitted the first batch of work which pulls across most of the changes to the ReadOptions struct. See #72

Sorry this took a while (had to leave my desk for a while). Here's my quick analysis of how the two forks have implemented static linking.
tl;dr The only notable difference between the two is how they both build RocksDB. I much prefer the approach taken in ethcore as it will be much easier to support different platforms with it. It also doesn't do any downloads during compilation.
What they both do

They both have a sub-crate suffixed with -sys for FFI bindings
They both use a build.rs file for compiling RocksDB
They both link against a specific version of RocksDB

What they do differently

ngaut downloads and compiles bz2, libz, lz4, snappy and RocksDB
ethcore bundles the RocksDB and snappy code in the crate and builds locally

My opinions:
I much prefer the way the ethcore fork works here. Performing extra downloads during a build feels very ugly to me, download links may break in the future and we can’t be sure the build server has an internet connection.
The ngaut fork builds with bz2, libz, lz4 and snappy whereas the ethcore fork only builds with snappy. This would obviously, improve compile time considerably but I wonder is snappy is sufficient for all use cases?
I don’t like having full copies of dependencies bundled in the repo, I think git-submodules would be a good fit here instead.

ngaut’s build.rs file calls a shell script which downloads each project, then builds them using their Makefile
ethcore’s build.rs uses the gcc-rs crate to perform the build (not using the project’s makefiles, or any external tools)

My opinions:
I prefer the approach the ethcore fork has taken here. Using a shell script would make cross-platform support very difficult. Different distributions of Linux may not have all the required tools installed to run it and there are many different shells to take into account. It will need to be completely reimplemented for Windows support.
Ethcore keeps all of it’s build logic inside the build.rs file. This file can be built and run on any platform that Rust supports. The gcc-rs crate works well with Visual C++ on Windows (the ethcore fork has Windows support).
The only downside I can find to the ethcore approach is that every .cc file needs to be listed in build.rs. This list may need to be updated when RocksDB is updated. The linker should give us an error message when this list is incorrect so there’s little chance of mistakes.
Please let me know if you disagree with my observations/opinions. The approach I'd like to take is to copy the implementation from the ethcore fork, tweak it to fix any compatibility issues and make it use git-submodules instead of bundling RocksDB/snappy. Please let me know if that sounds OK to you.

Actually we thought about using git submodule and gcc-rs at first. But it seems ugly to me to track all the .cc files again which was done in the Makefile already. And git-submodule is too slow for me to clone a new repository from the beginning.
I think almost all the linux distribution and OS X have bash support. Windows can use PowerShell get the job done, though It's not implemented yet in the ngaut branch.
If build server doesn't have a network connection, then it will be hard for it to build a rust project. Because the crates registry, crates source are needed to be updated (or initialized) via a connection to github.com and crates.io.
Please note that, gcc-rs just wraps a gcc process, so it depends on gcc compiler. Rocksdb can't be compiled without any external tools (yet).

Actually we thought about using git submodule and gcc-rs at first. But it seems ugly to me to track all the .cc files again which was done in the Makefile already. And git-submodule is too slow for me to clone a new repository from the beginning.
I think almost all the linux distribution and OS X have bash support. Windows can use PowerShell get the job done, though It's not implemented yet in the ngaut branch.
If build server doesn't have a network connection, then it will be hard for it to build a rust project. Because the crates registry, crates source are needed to be updated (or initialized) via a connection to github.com and crates.io.
Please note that, gcc-rs just wraps a gcc process, so it depends on gcc compiler. Rocksdb can't be compiled without any external tools (yet).

Thanks for chiming in @BusyJay!

And git-submodule is too slow for me to clone a new repository from the beginning.

Ahh, good point. Having to pull down a 6000-commit history when you only need the state at one time is a bit heavy. This won't be a problem when rust-rocksdb is installed from crates.io though as the checked out submodules will be bundled in the crate at the point of publishing it.

But it seems ugly to me to track all the .cc files again which was done in the Makefile already.

I agree, but it's the lesser of two evils in my opinion

If build server doesn't have a network connection, then it will be hard for it to build a rust project. Because the crates registry, crates source are needed to be updated (or initialized) via a connection to github.com and crates.io.

I'll elaborate on this a bit

It is possible that somebody would want to run a local registry (they may be going on an airplane, or work in a high-security environment). In this case, the user does not expect any network connections would be required to install the crate. See: http://doc.crates.io/source-replacement.html#local-registry-sources

When Cargo's --frozen flag is used, Cargo will refuse to run if it needs to make a network connection. So if Cargo did run, the user would expect that no network connections were made, but this isn't the case with the ngaut branch, as it stands, which will proceed to make four downloads at compile time. See: http://doc.crates.io/faq.html#how-can-cargo-work-offline

A lot of people cache the crate sources on their build server (including this project), but not the built versions. Every time they compile, they will have to refetch these four sources. They are only considered by cargo as only a side-effect of the build script and not something worth caching. This, again, is not what the user would expect.

It adds four build-time dependencies that are outside of the cargo ecosystem. If any of those sites they are hosted on go down, nobody will be able to build.

Would you consider a PR that changed the FFI module to something like https://github.com/alexreg/rocksdb-sys/blob/master/src/lib.rs? This is up-to-date with RocksDB (v4.11.2), and stays faithful to the C API – no renamings of types, and arguably more complete tests (see https://github.com/alexreg/rocksdb-sys/blob/master/tests/c_test.rs in the same repo, since they've been ported from the C tests). I'd also recommend forking the FFI stuff into a separate _sys crate, even if it lies within the same repo.

@alexreg I've forked the FFI module into a separate -sys crate in my "implement static linking" PR (https://github.com/spacejam/rust-rocksdb/pull/80) submitted earlier today. Hopefully this will be looked at soon!
Those FFI bindings are much more complete than what we currently have (implementing 274 library functions vs 116 in this repo), and the test suite is a massive bonus. I think pulling this in would be very helpful.
But I'd still like the common, everyday API to feel "rusty", following Rust's naming conventions. The FFI module could become a behind the scenes thing which follows RocksDB's naming conventions (and is public for those who want more control). The enums currently defined in FFI will probably have to be redefined somewhere else.

@kaedroho Thanks, that's good to know. Maybe if your PR gets accepted soon, and @spacejam is happy with this proposed reworking of the FFI bindings, then I could have a go implementing it on top of the latest changes.

@kaedroho How did you get librocksdb to link (statically) against libjemalloc by the way? Specifically, the version of libjemalloc.a that Rust uses.

Peeps, that is how open source collaboration should work! Thank you, you are awesome! :D

I'd like to continue to pull in changes over time from the various forks, so this is by no means over, but for now we've wrapped up a nice big chunk.
Today 0.5.0 will be released, which includes the great work you've all done in this issue, and others!
I've been pleasantly surprised by the amount of contribution that this issue has stirred up, and blown away by the amount of effort put in by @kaedroho and @alexreg ! Everyone who contributed, this project is in a much nicer place now after your efforts :) Thank you!

Glad to help. Thanks likewise for maintaining this project so diligently. :-)

On 20 Nov 2016, at 11:03, Tyler Neely<EMAIL_ADDRESS>wrote:
Closed #69 https://github.com/spacejam/rust-rocksdb/issues/69.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/spacejam/rust-rocksdb/issues/69#event-865579420, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEF3Mtu1brGHHFw4NxgJS6YIRgS6koiks5rACkPgaJpZM4KJmnu.


Fix Windows clang CI for CC toolchain resolution

Select the correct toolchain and platform for
building with clang on windows with CC toolchain
resolution active.
Attempting to fix #876

@comius @meteorcloudy I think this should fix the incompatible_enable_cc_toolchain_resolution build for this package.

Codecov Report

Merging #877 (0dfb5eb) into master (05fbc65) will increase coverage by 3.30%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #877      +/-   ##
==========================================
+ Coverage   73.29%   76.60%   +3.30%
==========================================
  Files          17       14       -3
  Lines        3292     2932     -360
==========================================
- Hits         2413     2246     -167
+ Misses        879      686     -193

Impacted Files
Coverage Δ

src/utilities.cc
65.57% <0.00%> (-2.31%)
:arrow_down:

src/base/mutex.h
100.00% <0.00%> (ø)

src/windows/dirent.h

src/windows/port.cc

src/windows/port.h

src/demangle.cc
97.59% <0.00%> (+0.11%)
:arrow_up:

src/glog/logging.h.in
80.30% <0.00%> (+0.30%)
:arrow_up:

src/signalhandler.cc
6.75% <0.00%> (+0.30%)
:arrow_up:

src/vlog_is_on.cc
70.31% <0.00%> (+0.31%)
:arrow_up:

src/logging.cc
75.65% <0.00%> (+0.74%)
:arrow_up:

... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Thanks, buildkite logs look good! Do you know if --incompatible_enable_cc_toolchain_resolution is already the default at head, or should we run a build explicitly setting --incompatible_enable_cc_toolchain_resolution to check this?

Do you know if --incompatible_enable_cc_toolchain_resolution is already the default at head, or should we run a build explicitly setting --incompatible_enable_cc_toolchain_resolution to check this?

It's false at head, you need to set it manually.

Many thanks, Ivo. I explicitly enabled it for a test build and it seems happy: https://buildkite.com/bazel/google-logging/builds/3010#0184c7ce-6aa4-4803-8e30-50ab4834352f


No OCSP responder URL in the certificate

Hello,
I'm trying your addon with a self-signed certificate and I'm getting this in the logs:
nginx: [warn] "ssl_stapling" ignored, no OCSP responder URL in the certificate "/ssl/my_selft_signed_certificate.pem"
Would it be possible to by-pass that OSCP check?
Thank you!

It is a warning that doesn’t effect the working. What is your question?

Yes, I noticed that I wasn’t affecting the working a bit later. Other than that, everything’s fine.
Thank you!
From: Franck Nijhof<EMAIL_ADDRESS>Sent: jeudi 28 mars 2019 19:26
To: hassio-addons/addon-mqtt<EMAIL_ADDRESS>Cc: ericdes<EMAIL_ADDRESS>Author<EMAIL_ADDRESS>Subject: Re: [hassio-addons/addon-mqtt] No OCSP responder URL in the certificate (#35)
It is a warning that doesn’t effect the working. What is your question?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/hassio-addons/addon-mqtt/issues/35#issuecomment-477717353 , or mute the thread https://github.com/notifications/unsubscribe-auth/AAE_NY75Y_Ae5im7E4RwRYu0HXkyuMkbks5vbQlAgaJpZM4cQVWX .  https://github.com/notifications/beacon/AAE_NUQ9l41G8AMlKQZmRBtEmFpebIGLks5vbQlAgaJpZM4cQVWX.gif

Check! Thanks for letting us know! 👍


Authorization Header does not exist in flask request object

I'm expecting Authorization and a request id in request header but it is missing.
print(request.headers)
Host: <IP_ADDRESS>:9000
Origin: http://localhost:9000
Accept-Encoding: gzip, deflate
Dnt: 1
Access-Control-Request-Headers: authorization,request_id
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Connection: keep-alive
Access-Control-Request-Method: GET
Accept-Language: en-GB,en;q=0.5
User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0

i have parameter header in my docstring
parameters:
  - in: header
    $ref: '#/definitions/Authorization'

i found the issue now. the issue is the preflight request. :)

What did you do to get this to work?


Best practice feature request: touch and wheel event listeners as passive

Feature request
Description
Would it be possible to have a prop to enable/disable the touch and wheel event listeners as passive?
It is recommended by Lighthouse

More info here
Browser Affected

[x] Chrome
[ ] Edge
[ ] Firefox
[ ] Helios
[ ] IE
[ ] Safari

OS

[ ] Android
[ ] iOS
[ ] Linux
[x] MacOS
[ ] Windows

Versions

model-viewer: v0.10.0
three.js: r115

The warning disapears  in Lighthouse if you are using the NPM package, instead of the Unpkg ones.


Request.is() does not work as expected if the path contains newline codes

Laravel Version
10.17.1
PHP Version
8.2.10
Database Driver & Version
No response
Description
The title says it all.
Steps To Reproduce
Accessing the following route with /hoge/fuga%0A/foo returns NG, but isn't it correct that this behavior should return OK?
Route::get('/hoge/{fuga}/foo', fn() => request()->is('hoge/*') ? 'OK' : 'NG');

By the way, accessing with /hoge/fuga/foo returns OK.

Since it is a path parameter, you never know what will be entered.
So shouldn't the behavior be problem-free even if a newline code comes in?


home-assistant: 2021.1.2 -> 2021.1.3

Motivation for this change
https://github.com/home-assistant/core/releases/tag/2021.1.3
https://www.home-assistant.io/blog/2021/01/14/security-bulletin/
Things done

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

[x] NixOS
[ ] macOS
[ ] other Linux distributions

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

Result of nixpkgs-review pr 109463 run on x86_64-linux 1

1 package built:

home-assistant

Result of nixpkgs-review pr 109463 run on x86_64-linux 1

1 package built:

home-assistant

That is expected. You'd have to use the module as it inherits additional packages depending on its configuration.

That is expected. You'd have to use the module as it inherits additional packages depending on its configuration.


[5.2] Allow XSRF (CSRF) cookie lifetime to be set from configuration

Currently you are unable to set the lifetime of the xsrf cookie. It is currently hard coded at 120 minutes here
This change will allow it to pull the value from the session configuration. Since the default value for 'lifetime' in the session config is also 120, this should be backwards compatible.

Unlike the session duration, we just need the CSRF token to be valid across two requests.

On a related note, refs #6518.

That is true. However, the issue I was running into was if the difference in time between those two requests is larger than 120 minutes, a token mismatch exception will be thrown on the next request. There is no way to control that margin since it hard coded in.

You could extend the VerifyCsrfToken middleware and reimplement the addCookieToResponse method.
Maybe better, the cookie duration could be moved to a class property or method, so you would just have to redefine this, instead of the whole VerifyCsrfToken method.

Extending the middleware is exactly how I solved it for myself, but I figured someone could run into this problem in the future, hence the pr.
I considered implementing the property way that you suggest, but it seemed a simpler fix to just use the configuration since it was already there. I can adjust the pr to utilize a property instead if its ultimately decided that this is worth doing.


ncp: Added support for PROP_THREAD_NETWORK_ID_TIMEOUT

This change adds support for a new Spinel property that maps to the otGetNetworkIdTimeout() and otSetNetworkIdTimeout() APIs.

LGTM :v:
Tested and confirmed both get/set work for this new property.

Apparently spurious test failure:
FAIL: thread-cert/Cert_5_5_06_SplitWeight.py

I've kicked travis.

LGTM :+1:

LGTM 👍


Issue-9

colab notebook for dev, test

This is also not a clean commit, because I branched off of issue 7, instead of main.
The colab notebook should be a separate branch.


fix: Emit RoutingCreated event for mediator routing record

Added RoutingCreatedEvent emit after creation of mediator routing record in MediatorService.
This change is required to enable mediator functionality for tenant agents as registerRecipientKeyForTenant will not be called for mediator routing key otherwise (done in RoutingCreatedEvent listener).

Do we want tenants to act as mediators? Do you have a specific use case?

Yes, we have a use case where we want to use mediator functionality in multi-tenancy mode.
It seems that in multi-tenancy mode it's not possible to normally use root agent as a mediator because of how TenantAgentContextProvider resolves context for inbound message (it will not find agent context for root agent as it does not have tenant routing record). So we decided to create a separate tenant agent for mediator functionality and it also seems to be more appropriate with current API.
We actually use this change in our project with patch-package as a quick fix for now. It works fine so far, so we would like to keep this approach for now. But thanks for clarification, we will keep possible issues in mind.


Add Extract Animation to ayon settings

Changelog Description
Resolve #66 and #36
This PR is to add Extract Animation to the ayon setting, and use apply_setting function into ExtractAnimation to set enabled state.

Additional info
Build maya addon with this branch
Testing notes:

Build Maya addon with this branch
Install Maya addon
Launch Maya
Publish Animation Product Type.

@BigRoy just as Im inspecting the Animation publish instance I wasnt able to get rid of USD and its settings on the publish instance - they are there even if explicitly turned off within Ayon settings. Are there any reasons for such behaviour? As now it just clutters UI (when USD not used)? thx for any insight!

@BigRoy just as Im inspecting the Animation publish instance I wasnt able to get rid of USD and its settings on the publish instance - they are there even if explicitly turned off within Ayon settings. Are there any reasons for such behaviour? As now it just clutters UI (when USD not used)? thx for any insight!

This is also my question too.
I find out that OptionalPyblishPluginMixin is actually inherited from AYONPyblishPluginMixin, we can probably use that, it's just that we need to implement the variable optional=False in ExtractAlembic.

Please double check whether all the regular Extract Alembic settings still also apply to the Extract Animation extractor. Like the writeUVSets, etc. and all the other instance toggles or settings toggles. I hope they do (and assume so too), but may need an actual production run test for that @LiborBatek

This is also my question too.
I find out that OptionalPyblishPluginMixin is actually inherited from AYONPyblishPluginMixin, we can probably use that, it's just that we need to implement the variable optional=False in ExtractAlembic.

Has this been resolved, or is that still an issue?

@moonyuet I've taken the liberty to push some changes. Please check whether you don't hate them. :)
I did face an issue when disabling all extractions and only enabling "Collection Fbx Animation" which I figured would make it export an FBX that I hit the error that no FBX got extracted in the end and the publish thus errored during integration.
DEBUG: Top group of animated skeleton not found in ['char_hero_rigMain_01_:rigMain_skeletonAnim_SET'].
Skipping fbx animation extraction.

publish-report-240827-23-03.json
We should capture that as validation somehow as well?
Other than that the logic works for me.

we should add validator to check on this. We have similar validator on rig.fbx families and we can do it with the animation.fbx too.

Im not able to launch maya due to failure when triggering it:
Traceback (most recent call last):
  File "C:\Work\REPO\ayon-core\client\ayon_core\tools\launcher\models\actions.py", line 405, in trigger_action
    action.process(selection)
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\action.py", line 126, in process
    self.application.launch(
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\defs.py", line 298, in launch
    return self.manager.launch(self.full_name, *args, **kwargs)
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\manager.py", line 173, in launch
    return self.launch_with_context(context)
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\manager.py", line 149, in launch_with_context
    return launch_context.launch()
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\manager.py", line 597, in launch
    self.run_prelaunch_hooks()
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\manager.py", line 581, in run_prelaunch_hooks
    prelaunch_hook.execute()
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\maya_0.2.11-dev.1\ayon_maya\hooks\pre_auto_load_plugins.py", line 19, in execute
    enabled = maya_settings[ "explicit_plugins_loading"]["enabled" ]
KeyERROR: 'explicit_plugins_loading'

seems like issue of the ayon-maya package I have built from this PR ???

Im not able to launch maya due to failure when triggering it:

Traceback (most recent call last):
  File "C:\Work\REPO\ayon-core\client\ayon_core\tools\launcher\models\actions.py", line 405, in trigger_action
    action.process(selection)
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\action.py", line 126, in process
    self.application.launch(
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\defs.py", line 298, in launch
    return self.manager.launch(self.full_name, *args, **kwargs)
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\manager.py", line 173, in launch
    return self.launch_with_context(context)
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\manager.py", line 149, in launch_with_context
    return launch_context.launch()
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\manager.py", line 597, in launch
    self.run_prelaunch_hooks()
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\applications_0.2.4\ayon_applications\manager.py", line 581, in run_prelaunch_hooks
    prelaunch_hook.execute()
  File "C:\Users\lbate\AppData\Local\Ynput\AYON\addons\maya_0.2.11-dev.1\ayon_maya\hooks\pre_auto_load_plugins.py", line 19, in execute
    enabled = maya_settings[ "explicit_plugins_loading"]["enabled" ]
KeyERROR: 'explicit_plugins_loading'

seems like issue of the ayon-maya package I have built from this PR ???

You sir, are correct!
2024-08-28 19:27:54 2024-08-28 17:27:54 ERROR      server          Unable to load maya 0.2.11-dev.1 settings
2024-08-28 19:27:54
2024-08-28 19:27:54     Traceback (most recent call last):
2024-08-28 19:27:54       File "/backend/api/settings/settings.py", line 175, in get_all_settings
2024-08-28 19:27:54         settings = await addon.get_studio_settings(variant)
2024-08-28 19:27:54                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-08-28 19:27:54       File "/backend/ayon_server/addons/addon.py", line 414, in get_studio_settings
2024-08-28 19:27:54         settings = await self.get_default_settings()
2024-08-28 19:27:54                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-08-28 19:27:54       File "/addons/maya/0.2.11-dev.1/server/__init__.py", line 12, in get_default_settings
2024-08-28 19:27:54         return settings_model_cls(**DEFAULT_MAYA_SETTING)
2024-08-28 19:27:54                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-08-28 19:27:54       File "pydantic/main.py", line 341, in pydantic.main.BaseModel.__init__
2024-08-28 19:27:54     pydantic.error_wrappers.ValidationError: 3 validation errors for MayaSettings
2024-08-28 19:27:54     publish -> enabled
2024-08-28 19:27:54       field required (type=value_error.missing)
2024-08-28 19:27:54     publish -> optional
2024-08-28 19:27:54       field required (type=value_error.missing)
2024-08-28 19:27:54     publish -> active

That was a bit of search which setting defaults were missing. But here we go. Should work now.

@BigRoy thx, works now (Maya launches)
There seems to be an issue for Extract Alembic Animation not being Optional aka visible in the Publisher

meanwhile as Im testing it atm

Generally it works but there is still no human readible failure message if no repre been produced (aka all extractors disabled)

Please share your publisher report in the cases it fails.

And as mentioned earlier...Extract Animation(Alembic) when set to Optional is not visible in the Publisher UI

Fixed with https://github.com/ynput/ayon-maya/pull/68/commits/845a3ca4f09b05c572d4009ecf9754192f3adabd

...and still no luck with optional Extract Alembic Anim switch....missing

@BigRoy went through the conversation and just to point that having Extract Anim (FBX) as optional can be really handy...as even if no failure due to empty skeletonAnim set (aka all occupied and be ready for output) there can be occasions when user doesnt want to produce any Animation (FBX) ...otherwise it might need to go to settings to disable it globally - big no no!

went through the conversation and just to point that having Extract Anim (FBX) as optional can be really handy...as even if no failure due to empty skeletonAnim set (aka all occupied and be ready for output) there can be occasions when user doesnt want to produce any Animation (FBX) ...otherwise it might need to go to settings to disable it globally - big no no!

I have never said it's not allowed to be optional ;) I've only stated that we shouldn't change what is enabled or not or what is optional or not just to remain backwards compatible.
The Optional state for the FBX animation is the "Collect Fbx Animation" plug-in, which you can enable (and/or make optional) in settings. It will (unfortunately) list as "Collect Fbx Animation" in the publisher UI as optional toggle. To change that around we should make a separate issue and maybe refactor some logic surrounding that.
Also, I've fixed the validation because it didn't check for the multiverse extractor correctly - I'm not sure why it did work for me before, but I was able to reproduce and fix it now.
So, test again! :) @LiborBatek

@BigRoy sry but I was more refering to miss optional knob for Extract Alembic Animation in the Publisher UI, and I still miss it!

Odd - it works for me.

Could you share a copy of  your ExtractAnimation and ExtractAlembic settings?


[DropdownMenu]: using Button component inside Trigger

Current Behavior
When I used html button inside the Trigger everything works fine, but when I used my Button component for some reason the Content will have transform: translate3d(0px, -200%, 0px)
Expected behavior
It should be the same behavior if I use the html button or my Button component
Reproducible example
CodeSandbox Template
Additional context
This is happening only if I used Trigger with asChild. In this case it is mandatory to be used like this because I can't have 2 buttons for the Trigger.
 This is so that your button can be measured in order to position the content.


feat(replica): RebuildRetryCount for disconnection

Do not count the RebuildRetryCount if rebuilding reused replicas failed caused by network disconnection.
Need to collect more failed reasons for rebuilding replicas.
longhorn/longhorn#1923
Regression test: https://ci.longhorn.io/job/private/job/longhorn-tests-regression/2680

@mantissahz :point_up: the PR check (codefactor) failed, need to fix that.

the PR check (codefactor) failed, need to fix that.

Yes, modified, but it was generated by k8s/generate_code.sh (zz_generated.deepcopy.go), it seems like it should not be chagned?

the PR check (codefactor) failed, need to fix that.

Yes, modified, but it was generated by k8s/generate_code.sh (zz_generated.deepcopy.go), it seems like it should not be chagned?

then it's embarrassing, we can just ignore this part we can't control.

close this PR and I will create a new one after refactoring the code with introducing a more general field like Condition


spacing issue on download button

When the window isn't very big, things overlap funny.

@brownsarahm For the current web framework, it is not fixable. Only using style.css to adjust the position will not solve this issue.


Missing material icons more_up and more_down

Use case
Maybe I am misunderstanding something, but I thought the icons.dart includes all material icons, but I found some that are missing.
Proposal
The more_down and more_up icons are absent from the material icons.

What you are looking at is a material symbol which is slightly different from material icons. Support for this is being tracked in https://github.com/flutter/flutter/issues/102560. In the interim, you can use https://pub.dev/packages/material_symbols_icons
Closing this as a duplicate of the issue linked above.


installing edge generates warning messages from webHints and offers no obvious recourse other than uninstalling it all

Windows 10
ext v1.4.0

Describe the bug:
Installing edge tools forces me to use WebHints. Eventually, I'll figure out how to stop annoying me by constantly warning me if I say "removeComments" false, but for now, I have to disable edge tools
Repro steps:
Install edge tools
find out that it is warning me that I don't have removeComments in my tsconfig and if I do add it, it warns me if I set it to false.
Expected behavior:
For it to mind it's own damn business and if not, give me a simple way to tell it to stop bothering me.
Additional context:
The byproduct of installing EdgeTools must not be a blizzard of annoying messages based on someone's notion of what code should look like. It could ask politely and I say, go away kid, it should go away.
I tried to keep this toned down but wreaking havoc on my code without simple remedies inline is a terrible imposition and (to use a very very mild word) impolite.

Thanks. I did a quick but didn't find it in a quick look. But I can wait for the UX upgrade
As an FYI I did ctrl-shift-p settings and looked at the various settings. Since VSCode is more a tool for me than a focus it's easier to wait than spend much time on it. Thanks.

I finally found that setting and was disappointed that it can't be refined yet. Please work on allowing adjustments to the webhints (error vs warning settings and turning particular hints on/off as needed). This is actually pretty great to have.
Thanks.

@darkgrayknight thanks for the feedback! While we don't have a UI for it yet, the webhint integration can be configured using webhint's existing config file format by adding a .hintrc file to the root of your project. See the webhint docs for full details: https://webhint.io/docs/user-guide/configuring-webhint/summary/
But for some quick examples, say you were getting this CSS report from webhint about backdrop-filter not being supported in all your target browsers:

You could turn off all CSS compat warnings with this .hintrc (pretty heavy-handed still):
{
    "extends": ["development"],
    "hints": {
        "compat-api/css": "off"
    }
}

Or you could adjust your target browsers to exclude browsers you don't care about (maybe Internet Explorer?) with this .hintrc:
{
    "extends": ["development"],
    "browserslist": ["defaults", "not IE 11"]
}

Or you could ignore just the backdrop-filter CSS property and still get feedback for others with this .hintrc:
{
    "extends": ["development"],
    "hints": {
        "compat-api/css": ["default", {
            "ignore": ["backdrop-filter"]
        }]
    }
}

Thanks for the tip about creating a .hintrc file, and the specific code for excluding IE 11 hints. That was just what I was looking for, and it does work on per-project basis. I'd prefer to set this in VSCode globally, rather than include a .hintrc file in every project. I see the webhint documentation for setting environment variables, but I wasn't able to get that to work. Has anyone else gotten this working?

Please fix.


[Improvement] create and alter failed since the operations in catalog are async

What would you like to be improved?
there is a reload action in object (table and topic) alteration and creation, which used to obtain some values generated by the underlying catalog

reload table in alteration:
https://github.com/datastrato/gravitino/blob/3a973abf63952dc3102b78c0684705ef6cee4dc0/core/src/main/java/com/datastrato/gravitino/catalog/TableOperationDispatcher.java#L234-L242

reload topic in creation:
https://github.com/datastrato/gravitino/blob/3a973abf63952dc3102b78c0684705ef6cee4dc0/core/src/main/java/com/datastrato/gravitino/catalog/TopicOperationDispatcher.java#L143-L148

The create and alter may be failed since the operations in the catalog are async:

#3317
#3496

How should we improve?
Here are three solutions I can think of right now:

remove the reload action in creation and alteration
If we choose to remove the reload operation, then we may need to refactor the API so that the return value is viod, otherwise it will result in an inconsistency with the manual load

add await timeout to the reload action
If the corresponding operation is not completed after the timeout, whether to return success or failure to the client is a point that needs to be considered.

@shaofengshi  @jerryshao  cc

Hi @mchades, I want to try to solve this issue.
I have done some code tracing from createTopic in TopicOperationDispatcher.
I think how about just make createTopic synchronous by using like createTopicsResult.all().whenComplete() in KafkaCatalogOperations and form new properties just like loadTopic implementation.
But it may have some performance issues, so just rewriting reload action to try and timeout is also a common solution. WDYT?

Hi @mchades, I want to try to solve this issue.
I have done some code tracing from createTopic in TopicOperationDispatcher. I think how about just make createTopic synchronous by using like createTopicsResult.all().whenComplete() in KafkaCatalogOperations and forming new properties just like loadTopic implementation. But it may have some performance issues, so just rewriting reload action to try and timeout is also a common solution. WDYT?

@noidname01  Sorry for the late reply.
What's the different between createTopicsResult.all().whenComplete() and current usage of createTopicsResult.topicId().get():
https://github.com/datastrato/gravitino/blob/282fc0d488177b7944c99fa2806dee16143f37fe/catalogs/catalog-kafka/src/main/java/com/datastrato/gravitino/catalog/kafka/KafkaCatalogOperations.java#L229
Does createTopicsResult.all().get() really ensure the synchronization of creation?
If this method can ensure synchronous creation, I think we can use it first, and performance issues can be considered in a separate PR.

@mchades Sorry for the late reply, too.
I've done some research in how Kafka creates the topics.
It seems there's no difference between createTopicsResult.all().whenComplete() and createTopicsResult.topicId().get(), because the Kafka server will return metadata all at once.
And even though the create method returns, the topic also needs to be propagated in Kafka system.
So there is an unmeasurable time diff between the time that the create method returns and the time the created topic can be described.
To really solve this, I think add retry mechanism on reload action might be a feasible solution.
WDYT?

@mchades Sorry for the late reply, too.
I've done some research in how Kafka creates the topics. It seems there's no difference between createTopicsResult.all().whenComplete() and createTopicsResult.topicId().get(), because the Kafka server will return metadata all at once. And even though the create method returns, the topic also needs to be propagated in Kafka system. So there is an unmeasurable time diff between the time that the create method returns and the time the created topic can be described.
To really solve this, I think add retry mechanism on reload action might be a feasible solution. WDYT?

Thanks for your feedback!
So the conclusions and solutions obtained at present are as I described in the description and you tend to prefer option 2, right?

How should we improve?
Here are three solutions I can think of right now:

remove the reload action in creation and alteration
If we choose to remove the reload operation, then we may need to refactor the API so that the return value is viod, otherwise it will result in an inconsistency with the manual load

add await timeout to the reload action
If the corresponding operation is not completed after the timeout, whether to return success or failure to the client is a point that needs to be considered.

Yes, you're right.

@noidname01
After discussing with @jerryshao offline,  it is better to only return the properties passed by the user during creation. This change allows us to eliminate the reload action from the creation process.

@noidname01 After discussing with @jerryshao offline, it is better to only return the properties passed by the user during creation. This change allows us to eliminate the reload action from the creation process.

So it is basically option 1 above but the return value is changed from void to Topic created using properties passed by the user during creation, instead of getting properties from reload action.

@noidname01 After discussing with @jerryshao offline, it is better to only return the properties passed by the user during creation. This change allows us to eliminate the reload action from the creation process.

So it is basically option 1 above but the return value is changed from void to Topic created using properties passed by the user during creation, instead of getting properties from reload action.

yeah. BTW, not only for creating topic, but the table and fileset are also need to bechanged

Hi @noidname01 , are you still working on this? Do you need any updates or assistance?

@mchades Yeah, but I'm busy solving other issues like: #4012 and #3755
I've tried the solution we've been discussed, but the integration test would fail because there are some properties need to be filled by the server (like PARTITION_COUNT and REPLICATION_FACTOR in KafkaTopic), so I will work on modifying the tests if this solution is acceptable.

@mchades Sure, will work on this and finish ASAP.

Hi @noidname01 , Is there any progress on this issue recently?
I would like to have it completed by the end of this week. If you have a draft locally, you can submit a PR, and then I can build on your work.

@mchades Sorry for the inactiveness in this PR.
I've created a draft PR, I have done the main logic modification, the remaining to-do works is modifying the logic of ITs, which still use reload logic.


fixed issue with calculating sqrt for classification

fix for the issue with 0 selected features, the root cause is that under icx daal::internal::Math<float, sse2>::sSqrt(1) returns 0.9999.

/intelci: run


Integrate DNS resolution for docker-machine instances

It would be great if docker-machine would setup hostnames for each created docker machine instance.
docker-machine create registry ...
would create a hostname on the host machine: registry.dockermachine or registry.dm for short.
That would simplify accessing services, started by Docker containers inside the docker-machine instance.
A simple solution would be the feature, provided by the Node.js tool hostile:
https://github.com/feross/hostile

We've thought about it -- my strong feeling is that it's quite out of scope for Machine in its current form (and trickier to get right than it looks).  Might be something that we visit if we implement any kind of plugin / "hook" model.