Duplicate CSS using @import and require()

If you want access to LESS variables, you have to use @import.  LESS variables are not available from one require() to the next (see "Simple Example" section, at the very end).
This means each require() creates its own LESS compiling context, and if you want to use variables between .less files, you have to use a single require() statement, and use @import to pull in the files.
This is very counter-intuitive to the traditional npm system, where we can define a dependency and require() it, and it requires() its deps, (in this case, LESS styles).  Instead of using require() for each component's .less, we have to edit our global .less bundle, and manually @import everything separately from the JavaScript.

This isn't impossible, just a little extra work.  And, all modules would have to follow this pattern (of not requiring() their own .less, and instead allowing the end user to pull it in with @import).  If you're the author of all your components, that's possible.  Anyway, it's not a great solution, but here's how that might work:
Solution A:  Manage LESS and JS separately
Maintain a single global.less file, and make sure to only require it once.  In global.less, use @import to pull in all the individual .less files, where ever they may be (local or in node_modules).  For example,  @import "~/path/to/nav.less".
Also, in order to render the nav component's HTML, we'd need to require() the necessary JS: require("path/to/nav"), wherever that would normally happen.
** How it should work: **
We require("path/to/nav");, and inside the nav component, which then require("./nav.less"), which can than @import mixins that have already been @imported, without creating duplicate CSS.
Solution B:  Keep vars/mixins in separate files without any CSS output
LESS mixins and variables aren't output to CSS anyway - so if we isolate all our LESS vars and mixins in a separate file, it's OK if they are improperly required() and @imported a million times.  The mixins and vars get stripped.
The problem here, is that most of my mixins aren't abstract - they are real CSS rules, that I also use as mixins.  For example:
I add a CSS class to an element as a mixin/override
nav.addClass("large-padding"); // we need the real CSS rule defined

Or, I use it as a LESS mixin:
.nav {
    .large-padding;
}

Then we have to jump through some hoops to isolate the mixins from the actual style rules.  Normally, you can just do:
.large-padding {
    padding: 2em 2.4em;
}

And this gets output to the final CSS file.  But, you could do:
.large_padding() {
    padding: 2em 2.4em;
}

That defines a mixin only that does NOT get output (based on the ()). In this approach, you would keep this in a separate file that can safely be included in a way that results in duplicate-output, as long as we don't have any output in that file.
So then in another file, we can repeat the mixin as a real class:
.large-padding {
    .large_padding();
}

Then we can @import the mixins-only file anywhere (no matter how it was originally required()), without any duplicate output.
Of course, this is a lot of extra work.  I only came to this solution while I was typing this, and then realized that I use mixins as real classes most of the time, for quicker in-browser prototyping.  And so, to get this to work requires a lot of extra code.
Simple example: @import vs require():
Consider the following setup:
one.less:
@something: blue;

two.less:
.class {
    .something();
}

and then in our JavaScript:
require("./one.less");
require("./two.less");

This throws an error
The variables defined in one.less aren't available in two.less.  It seems each require() creates a separate LESS compiling scope.

Note: I'm using the extract text webpack plugin - not sure if that changes any of this

@import, however, works
In a root.less file:
@import "one.less";
@import "two.less";

and then w/ JavaScript:
require("./root.less");

This works - there is a single require() statement, and the two .less files are pulled in with @import statements, so there is a single LESS compiling scope, and the variables are available from one.less to two.less.
The problem is that if you require("./root.less"); in your bundle (which pulls in one.less and two.less), and then require("./another.less"); in which you need access to vars defined in one.less or two.less, you need to @import them (even though they've already been require().  This creates duplicate CSS in your final ouput.

Probably a similar issue:
https://github.com/webpack/less-loader/issues/100

Conclusion:  I want to author/publish/require() npm modules that can extend other npm modules.  This is possible with JavaScript.  To do this with LESS means we need to be able to @import

This article discusses this topic, for additional reading.
I'm still trying to figure out how I'm going to proceed...

Update:  I'm going to try Solution B described above.  Read the update I added in the original post... Ask me if you're interested and can't make sense of it.

Update, I stumbled upon this SO post and realize that this ability to @import as a reference only solves some of my challenges.
Now, my approach will be something like this:
/components/item/
    item.js
        require("./item.less");

/components/item2/
    item2.js
        require("./item/item.js");  // this adds item.less to the CSS output, if not already required()
        require("./item2.less");
    item2.less
        @import (reference) "../item/item.less"; // provides all LESS vars/mixins

This would normally cause duplicate CSS output, if you didn't use the (reference) feature.  The (reference) flag just tells LESS to import all the vars/mixins with 0 output.
I have yet to test this, but thought I'd keep a log of my journey.

@mikelew42 last post is a great solution and to me is a very clean one.
this way can add separated less for each component and @import variables and mixins but without the duplicated output.
thanks!

Duplicate of #7


Add length check for dirsDepth

This is a simple addition of a few bounds checks to ensure that the dirsDepth array is not accessed at an index beyond its size.
Type of change

Improvement (improvement to code, performance, etc)

Description
This change adds index checks before dirDepths[collectionIndexDirDepth] and dirDepths[collectionNameDepth] to avoid indexing beyond the end of the array. We could just check for the collectionIndexDirDepth and avoid the other two, but that would be more brittle in case the constants defined a few lines earlier were reordered. We could also just calculate len(dirsDepth) once as a small perf improvement.
Additional details
I have never used Hyperledger, and did not test this change in any way. I also don't know how likely it would be for dirDepths to short enough to cause the panic.
Related issues
This is related to the suggestion from @manish-sethi in hyperledger#1080.

@scovetta We appreciate your time and the effort you've invested in opening this PR. Unfortunately it's been a month since we've heard from you, we're unable to merge commits without a DCO signoff, and there are pending comments so I'm going to close this out. If you find that you have some time to to revisit this in the future, please feel free to reopen.
Thanks.


Individual request isolation

Closing this as this would be a major undertaking as stated and is not worth it for now just for a few options.


Feature request: manually run precompilation code during testing

It would be great if there was a way I could manually trigger the code within @setup_workload to run, outside of precompilation. The primary use-case for this would be to run the precompilation code during testing so it shows up on code coverage, and so that some precompilation errors show up as errors during testing.
My current workaround is to call a function containing all my precompilation code, with mode=:precompile -> @setup_workload, and mode=:compile -> (run as-is). https://github.com/MilesCranmer/SymbolicRegression.jl/blob/641180a561e350918e3fd6a8fd7c898d31bef11b/src/precompile.jl#L34
macro maybe_compile_workload(mode, ex)
    precompile_ex = Expr(
        :macrocall, Symbol("@compile_workload"), LineNumberNode(@__LINE__), ex
    )
    return quote
        if $(esc(mode)) == :compile
            $(esc(ex))
        elseif $(esc(mode)) == :precompile
            $(esc(precompile_ex))
        else
            error("Invalid value for mode: " * show($(esc(mode))))
        end
    end
end

with the function:
function do_precompilation(; mode=:precompile)
    @maybe_setup_workload mode begin
    ...
end

so I can have do_precompilation() in my precompilation code, and do_precompilation(; mode=:compile) in my testing code.
However this seems like it would be of common interest so it would be great if there could be a way I could force @compile_workload code to execute outside of precompilation.

Brought up in https://github.com/MilesCranmer/SymbolicRegression.jl/pull/198 w/ @timholy

I don’t know the internals, so this could be completely off base. But what if @setup_workload overloaded a function of PrecompileTools.jl with the code passed to the macro, in addition to the normal precompilation generation?
For example, what if it generated of the following code?
import PrecompileTools: test_precompilation

# Overloads this function:

function test_precompilation(::Val{MyPackageName})
    # contents passed to setup_workload
    # “@compile_workload” is converted to the identity
end

Then, you could test this by calling:
using Test, MyPackageName, PrecompileTools

test_precompilation(MyPackageName) # mapped to Val() internally

which, given that you’ve imported the package, will execute the code within the overloaded function. And if needed you could test the precompilation of any other loaded packages too.
The macro would remove @compile_workload from the contents of test_precompilation, leaving the inner code block naked so it executes when the function is called.

One orthogonal idea: you would want to have a keyword argument to @setup_workflow to explicitly turn this behavior on. For example:
@setup_workload testgen=true begin

end


Update scene_manager.rst

fixes typos referring to "open_xr_" - instead of "openxr_" signals in example snippet; now if no spacial env is found on the headset, one will be requested by this code.

Thanks for the fix!


Update howto-conditional-access-policy-block-legacy.md

The Client apps (preview) section needs to be updated to reflect the new options

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

#sign-off


[SUGGESTION] please set config:timeout default to false

in a fresh installation, with default setting we get:
Deprecated: The "timeout" configuration is deprecated and will be removed with the next major update. Set the "timeout" configuration to false and if needed use the set_timeout_limit() function in your own code. in ..vendor/rakibtg/sleekdb/src/Store.php on line 748
but if one use the default settings, there should no deprecation message.
now we have to supply:
$configuration = [
				'timeout' => false, // suppres deprecation message
			];

suggested fix:
class Store
{
...
  protected $timeout = 120;```
To
`protected $timeout = false;`

Hi @SegFaulty
Thank you for your suggestion. We intentionally didn't change the old default setting, because that would be a breaking change. The timeout configuration will be removed with the next major update. Until then we need to ensure backwards compatibility.

The problem here is that you are testing for timeout = false in the configuration, which is triggering a deprecation error even if we don't set a timeout value.
IMO, this isn't backwards compatible at all, as a deprecation error is not the previous behaviour.


[Android] Dex cannot parse version 52 byte code

When including:
compile 'org.tensorflow:tensorflow-lite:0.1.1'
I get the error:

[Android] Dex cannot parse version 52 byte code

If I use AGP 3.0.0+ the problem goes away since it has support for Java 8, but the problem exists if I try to use 2.3.3.

By AGP do you mean Android Studio? TensorFlow Lite requires Android Studio 3 (from here).
FYI @aselle @andrehentz


preparation to ping author on MR

Porting what we do in qr.

Corresponding QR change is here https://github.com/app-sre/qontract-reconcile/pull/4618


Laboratory work #1, Merkulova Ekaterina - 19FPL2

Nicely done! Keep on going. Start.py and lint to go

Overall comment:
Nicely done! Just rewrite some styling issues and be prepared to answer questions on October 3th

And please, return all docstrings. Do not remove them as they are a part of project

done


Spark 401353 fix device managment unit tests

fixed unit test for the tracks

Tests are passing locally but does not run on forks as it needs NPM_TOKEN for effects


Show loading indicator in listing view when content is loading

@iFlameing pls rebase volto.pot.

@tisto I will do it tomorrow


Problem with pushReplacementNamed

Hi all.
I am developing an App in Flutter (2.2.0) and I have encountered a problem when executing pushReplacementNamed.
Normally it works fine, but sometimes, I have detected through Crashlytics that it gives an error, specifically the following:
Fatal Exception: io.flutter.plugins.firebase.crashlytics.FlutterError: Null check operator used on a null value. Error thrown null.
       at .#0 Navigator.of (package:flutter/src/widgets/navigator.dart:2740:21)
       at .#1 Navigator.pushReplacementNamed (package:flutter/src/widgets/navigator.dart:1853:22)

Has this ever happened to you?
Do you know what could cause this error?

@adriancsbna
Are you able to replicate it locally in non-prod environment ?
Is it possible for you to provide a minimal complete reproducible code sample that triggers that error ?
Please provide as much information as you can.
Thanks.

Without additional information, we are unfortunately not sure how to resolve this issue.
We are therefore reluctantly going to close this bug for now.
Please don't hesitate to comment on the bug if you have any more information for us; we will reopen it right away!
Thanks for your contribution.
Could everyone who still has this problem please file a new issue with the exact description of what happens, logs, and the output of flutter doctor -v.
All system setups can be slightly different, so it's always better to open new issues and reference related issues.


Mechanism to deprecate modules

Currently, the module system supports adding new modules starting from a specific version. However, it will expect every module to be there on every new version so we need a deprecation mechanism associated to a specific version

@aguillenv has implemented this in #1412


[Service Bus] Consider consolidating getReceiver and getSessionReceiver functions

From offline discussions post the API review notes in #1481
We currently have 2 functions on a QueueClient/SubscriptionClient to get a receiver based on whether the Queue/Subscription supports sessions or not

getReceiver
getSessionReceiver

The receiver from getReceiver doesnt work with session enabled Queue/Subscription and the one from getSessionReceiver doesnt work with Queue/Subscription with no sessions enabled.
So, it is not really helpful to expose both to the user when they can only use 1 at a time. And it is not very simple to realize that they are using the wrong one. We rely on error message from the service because we are not aware of whether Queue/Subscription has sessions enabled or not.
It also opens the can of worms for the name getSessionReceiver. What does a session receiver receive? Sessions or messages?
Proposal:

Consolidate both and name the function createReceiver
Input arguments to this would be:

receiveMode
sessionOptions (optional, only if you are using session enabled queue/subscriptions)

sessionId: string | undefined
maxSessionAutoRenewLockDurationInSeconds | number

Returns

Receiver | SessionReceiver

Pros

Single method !!
Simple to configure to use sessions

Cons

Since return type is a union, only common functions show up in completion without casting by the user?

What about functions that have the same name in Receiver and SessionReceiver?

renewLock for sessions should be renamed to renewSessionLock
auto renew lock for message does not apply for sessions, how do we get around this?
receive for Non Sessions takes 1 extra option in the options bag compared to receive for Sessions

Updated Proposal:
  public createReciever(recieveMode: ReceiveMode): Receiver;
  public createReciever(
    receiveMode: ReceiveMode,
    sessionOptions: { sessionId: string; maxSessionAutoRenewLockDurationInSeconds?: number }
  ): SessionReceiver;
  public mygetReciever(
    receiveMode: ReceiveMode,
    sessionOptions?: { sessionId: string; maxSessionAutoRenewLockDurationInSeconds?: number }
  ): Receiver | SessionReceiver {

  }

I hope I am allowed to comment on this :)
How will you identify if a Queue/Subscription is session enabled? Currently the onus is on the developer to make that decision. Will the method work if I don't specify a sessionId in sessionOptions when fetching messages from a session enabled Queue/Subscription?

@gmantri Of course, we appreciate any feedback and discussion on any of the issues logged in this repository.
At present, we don't have a way in the library to identify if a Queue/Subscription is session enabled or not.
Until we do, below would be the behaviour

sessionOptions
Is session enabled on Queue/Subscription
behaviour

not passed at all
No
Normal receiver is created

not passed at all
Yes
Error

passed with no sessionId
No
Error

passed with no sessionId
Yes
Receiver is created for a random session as picked by Service Bus

passed with a sessionId
No
Error

passed with a sessionId
Yes
Receiver is created for given sessionId. Error thrown if a receiver already exists for this sessionId

In the error scenarios listed above for Queue/Subscription with sessions not enabled, we could look into intercepting the error in the library and returning a normal receiver.

Done with #1864
Logged follow up action item : #1892

Re-opening to track #1991

Done with #1991


Configurations for adaptive callChoreo connection timeouts

Configurations for adaptive callChoreo connection properties needs to be configured separately from the other common connection properties, so those can be modified from the deployment parameters(deplyment.toml)

Related PRs

https://github.com/wso2-extensions/identity-conditional-auth-functions/pull/126
https://github.com/wso2/carbon-identity-framework/pull/4477
https://github.com/wso2-enterprise/asgardeo-deployment-helm-is/pull/711

Related PRs

https://github.com/wso2-extensions/identity-conditional-auth-functions/pull/126
https://github.com/wso2/carbon-identity-framework/pull/4477


Fix missing headers, other small source problems.

Thanks for the pull request @kring!

:heavy_check_mark: Signed CLA found.

Reviewers, don't forget to make sure that:

[ ] Cesium for Unreal Samples works.


NTR: mitochondrial cloud

Can I have a new CC term please:
cytoplasm
--<mitochondrial cloud ; GO:NEW
exact synonym: Balbiani body
A prominent mass in the cytoplasm of previtellogenic
oocytes. The cloud contains both mitochondria and
electron-dense granulofibrillar material (GFM) and is
the source of germinal granule material.
PMID: 6541166
(for the annotation of Xpat: O73622)
thanks,
Becky
Reported by: beckyfoulger
Original Ticket: "geneontology/ontology-requests/3217":https://sourceforge.net/p/geneontology/ontology-requests/3217

thanks www.movied.org


fix: prevent Orisa shield from randomly disappearing - [merged]

Merges orisa-shield -> staging
Looks like the bug was caused by the fact that the player closest to reticle isn't always the player that ends up taking the damage.
In this case, the amount of damage would be regular but the rule condition would think that the damage was dealt to the target closest to reticle.

mentioned in commit 6623567db497b3730d2bc32f0995940f298b842e


MariaDB update

Thanks @dbart. This is now part of https://github.com/docker-library/docs/pull/1889, please note it in the PR when the next MariaDB/mariadb-docker release happens.


WIP: Prototype of MultiPlotter

I have been experimenting with the new PyVistaQt class MultiPlotter to bring proper SSAO and/or EDL to MNE on https://github.com/mne-tools/mne-python/pull/8771#issuecomment-765449184 (among other features like multiple toolbars, menu bars and multi-renderers). And I don't think it's reasonable to do that in one go.
This PR is far from being mergeable but I believe it could drive smaller PRs that can help with 3d backend refactoring in general. For instance:

[ ] Separate the concepts of "plotter" and 3d "viewer"
[ ] Get rid of self.plotter in Brain and use self._renderer instead
[ ] Get rid of self.plotter in _Renderer and use self.figure instead
[ ] Create proper Renderer bindings for add_mesh and add_actor
[ ] Generic way to pick a 3d viewer
[ ] Generic way to subplot
[ ] Generic way to iterate through the 3d viewers

I would add to this list, if we haven't done it already:

[ ] Avoid using PyVista's toolbar and menu bar and create/use our own as needed

Done in here on this PR:
https://github.com/mne-tools/mne-python/blob/f20f8161e4596318619b665860ccb02a2c6628c2/mne/viz/backends/_pyvista.py#L82-L83

Update after dd0d67ddc03e34fa8efd5fd07efe5e65166b9149 (with experimental margin=False):

I see the main challenges being 1) ensuring correct size and 2) managing picking nicely for all the viewers

Can you check to make sure volumetric rendering looks okay? Just want to make sure it plays nicely with the ambient occlusion etc.

There are some issues with the colorbar:
BackgroundPlotter has no attribute named scalar_bar

SSAO=on
SSAO=off

import os.path as op
import mne
data_path = mne.datasets.sample.data_path()
sample_dir = op.join(data_path, 'MEG', 'sample')
subjects_dir = op.join(data_path, 'subjects')
inv = mne.minimum_norm.read_inverse_operator(op.join(
    sample_dir, 'sample_audvis-meg-vol-7-meg-inv.fif'))
    # sample_dir, 'sample_audvis-meg-oct-6-meg-inv.fif'))
evoked = mne.read_evokeds(op.join(sample_dir, 'sample_audvis-ave.fif'))[0]
evoked.apply_baseline((None, 0)).pick_types(meg=True)
stc, res = mne.minimum_norm.apply_inverse(
    evoked, inv, return_residual=True, pick_ori='vector')

initial_time = 0.1
# stc, kwargs = stc, dict(overlay_alpha=0.5, brain_alpha=0.5, vector_alpha=0.)
stc, kwargs = stc.magnitude(), dict(alpha=0.5, surface='white')
kwargs['src'] = inv['src']
meth = stc.plot_3d if isinstance(stc, mne.VolSourceEstimate) else stc.plot
brain = meth(
    subjects_dir=subjects_dir, initial_time=initial_time,
    clim='auto', hemi='both', # views=['sag', 'axi', 'cor'],
    view_layout='horizontal', smoothing_steps='nearest', verbose=True,
    time_viewer=True, size=(800, 600), background='k',
    show_traces=False,
    **kwargs)

and plot_beamformer_lcmv:

It's better after #9121

This PR now requires https://github.com/pyvista/pyvistaqt/pull/90.
I plan to move the mplcanvas to a bottom dock and use the new resize_content() function to replace ensure_minimum_size.

In my experiments, the mplcanvas as a dock has several advantages:

it can be made separate dynamically with the default dock features (meaning that the "separate" particular case can be removed)
it does not mess with the central widget layout meaning that the resize_content() function is enough the ensure the viewer dimensions (important for screenshot). I have to demonstrate that by updating testing.
the size of the dock can be handled separately

This is how it looks locally:

Probably the margins can be nullified.

Agreed docking is useful, can that be a first PR, separate from the MultiPlotter stuff? It seems orthogonal, easier to review, and will get in sooner than if we tie it into this PR

Circle does not seem ready yet. I'll investigate locally:
RuntimeWarning: No 3D backend found. Resorting to matplotlib 3d


kapp does not validate resources before sending to k8s api (e.g. extra unknown keys)

Problem
When deploying with kapp, additional json keys that are not part of a K8s object's spec are automatically removed without errors or warnings.
This is a problem because erroneously specified k8s objects are not detected.
Example:
apiVersion: v1
kind: ConfigMap
annotations: # should be inside 'metadata'-block
  wrong: "ignored"
metadata:
  name: "test-config"
data:
  key: "value"

This is an actual problem that is continuously occurring with cf-for-k8s, e.g.
https://github.com/cloudfoundry/cf-for-k8s/issues/102
and
https://github.com/cloudfoundry/metric-proxy/pull/2.
Expected behaviour
Kapp should not silently remove keys from an object's spec. Kapp should return an error from the k8s API server instead.

Kapp should return an error from the k8s API server instead.

@jkbschmid this isnt a kapp behaviour but rather k8s api. it drops unkown keys without an error.

Oh, that's actually correct, good point. It is kubectl that does the validation, e.g.
$ kubectl apply -f /tmp/cm.yaml
error: error validating "/tmp/cm.yaml": error validating data: ValidationError(ConfigMap): unknown field "annotations" in io.k8s.api.core.v1.ConfigMap; if you choose to ignore these errors, turn validation off with --validate=false

I still think that validating against extra keys would prevent a lot of bugs in the future, though.

i hope that k8s api eventually will take ownership of such validations. im not sure this functionality  belongs in kapp vs a separate standalone tool. coincidentally ive been recommending folks to use kubeval for this purposes. below im running it against cf-for-k8s repo:
$ ytt -f config -f /tmp/values.yml | kubeval --strict --ignore-missing-schemas | grep -v PASS | grep -v "was not validated against a schema"

WARN - Set to ignore missing schemas
WARN - stdin contains an invalid Deployment - annotations: Additional property annotations is not allowed
WARN - stdin contains an invalid DaemonSet - strategy: Additional property strategy is not allowed

may be we can work with @garethr to improve kubeval in terms of making resources in error messages more identifiable, and/or making it possible to propagate input so that commands like this are possible: ytt -f ... | kbld -f- | kubeval | kapp deploy -a ...

I'd definitely be open to kubeval and conftest working with the piped input/output used by the k14s tools.
I've had less time to maintain kubeval recently unfortunately, but open to PRs. I'm planning on catching up on some basic maintenance soon.

I opened https://github.com/cloudfoundry/cf-for-k8s/pull/156 to use kubeval to validate the resources.


No imports directory

In your description you write: "Copy directory imports to your project location"
But I don't find the folder imports in your repository.
How can I include QtKnobs into my project?

Hi,
You will need to compile the project first. After compiling the "imports"
directory will be in your build directory.
Thanks,
Ashish
On Tue, Jul 17, 2018 at 4:21 PM, NelDav<EMAIL_ADDRESS>wrote:

In your description you write: "Copy directory imports to your project
location"
But I don't find the folder imports in your repository.
How can I include QtKnobs into my project?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/ashish157/QtKnobs/issues/9, or mute the thread
https://github.com/notifications/unsubscribe-auth/AFusA0SjPebyTGhZyIGVwu-RxecNU6r0ks5uHcHLgaJpZM4VSnwg
.

I already tried that befor. I didn't found the imports directory, because I compiled into a shadow directory. I hadn't know that.

If compilation is successful the imports directory will be created where it
is supposed to be created which in your case is shadow directory.
On Wed, Jul 18, 2018 at 1:08 PM, NelDav<EMAIL_ADDRESS>wrote:

I already tried that befor. I didn't found the imports directory, because
I compiled into a shadow directory. I hadn't know that.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/ashish157/QtKnobs/issues/9#issuecomment-405839328,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFusAy_V7TeyOKHtw8UoSSI1jbVgOI2dks5uHuX1gaJpZM4VSnwg
.


Default PinotAdmin Commands to not use System.exit(...).

This will default not to do any System.exit(), only when you set the env params.
E.g.
JAVA_OPTS="-Xms4G -Xmx4G -XX:MaxDirectMemorySize=30g -Dlog4j.configuration=conf/pinot-admin-log4j.properties -Dpinot.admin.system.exit=true" bin/pinot-admin.sh AddSchema -schemaFile ~/my/path/to/schema/schema.json -controllerHost localhost  -controllerPort 9000  -exec
Result:

-> % echo $?
1

Codecov Report

Merging #4110 into master will increase coverage by 20.91%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master    #4110       +/-   ##
=============================================
+ Coverage     45.74%   66.66%   +20.91%
- Complexity        0       20       +20
=============================================
  Files          1037     1037
  Lines         51426    51426
  Branches       7192     7192
=============================================
+ Hits          23525    34282    +10757
+ Misses        25891    14780    -11111
- Partials       2010     2364      +354

Impacted Files
Coverage Δ
Complexity Δ

...er/validation/BrokerResourceValidationManager.java
25% <0%> (-56.25%)
0% <0%> (ø)

...he/pinot/core/query/pruner/ValidSegmentPruner.java
57.14% <0%> (-28.58%)
0% <0%> (ø)

...apache/pinot/common/metrics/ValidationMetrics.java
20.28% <0%> (-23.19%)
0% <0%> (ø)

...r/validation/RealtimeSegmentValidationManager.java
55% <0%> (-20%)
0% <0%> (ø)

...e/impl/dictionary/LongOnHeapMutableDictionary.java
82.22% <0%> (-11.12%)
0% <0%> (ø)

...impl/dictionary/FloatOffHeapMutableDictionary.java
83.63% <0%> (-9.1%)
0% <0%> (ø)

.../impl/dictionary/FloatOnHeapMutableDictionary.java
86.66% <0%> (-6.67%)
0% <0%> (ø)

...mpl/dictionary/DoubleOffHeapMutableDictionary.java
65.45% <0%> (-5.46%)
0% <0%> (ø)

...not/broker/broker/helix/ClusterChangeMediator.java
66.66% <0%> (-3.45%)
0% <0%> (ø)

...e/operator/dociditerators/MVScanDocIdIterator.java
60.6% <0%> (-3.04%)
0% <0%> (ø)

... and 519 more

Continue to review full report at Codecov.

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


fix: edit text retention in EditProfileFragment

Details:
I checked if the EditText already contains text -> if it is then it shouldn't reset the text when rotating
Fixes #652
Screenshots for the change:

Please align your commit messages with the guideline


aten::pow support

Description
Improves some debugging tools and implements an aten::pow evaluator.
Partially addresses #833
Fixes: #923
Type of change
Please delete options that are not relevant and/or add your own.

New feature (non-breaking change which adds functionality)

Checklist:

[x] My code follows the style guidelines of this project (You can use the linters)
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas and hacks
[x] I have made corresponding changes to the documentation
[x] I have added tests to verify my fix or my feature
[x] New and existing unit tests pass locally with my changes

@narendasan
https://github.com/NVIDIA/Torch-TensorRT/pull/932 has some logging changes too as follows
In debug mode, we just list out the operators which are unsupported.
For people really interested in knowing which pytorch source code line is responsible for unsupported op, they need to call conversion::VerifyConverterSupportForBlock(g->block(), false) (suppress_errors=False). Let me know what you think ?

The unsupported ops will still get printed here, just at a lower severity


Exception::CallStack: avoid allocations in LibC.dl_iterate_phdr callback

Calling self.read_dwarf_sections directly from the C callback may lead to reallocations and deadlocks due to the internal lock held by dl_iterate_phdr (#10084). Work around this by storing object base address and passing it to self.read_dwarf_sections later when dl_iterate_phdr returns.
This also fixes #12329 because there's no need to disable GC around dl_iterate_phdr anymore.

Is it OK to only call read_dwarf_sections with the last address provided by dl_iterate_phdr? IIUC, that's what this change is introducing. Before it was calling read_dwarf_sections for each object loaded.

@beta-ziliani No, the original code was also stopping after reading the first object. Quoting dl_iterate_phdr(3):

The dl_iterate_phdr() function walks through the list of an
application's shared objects and calls the function callback once
for each object, until either all shared objects have been
processed or callback returns a nonzero value.

Totally missed that, thanks!


Page not updated in alignment with Kubernetes SIGS

Hi,
When looking at this readme is looks like the CSI blob driver is in preview mode.
But in kubernetes sigs this is saying it is GA.
What is the current status of CSI blob driver? is it in GA or in Preview mode?

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

ID: e3d2e0b2-cba5-c6ca-fc37-100f3982eec0
Version Independent ID: 6a0b4a02-a46d-8cd4-360b-54d77ff554b3
Content: Use Container Storage Interface (CSI) driver for Azure Blob storage on Azure Kubernetes Service (AKS) - Azure Kubernetes Service
Content Source: articles/aks/azure-blob-csi.md
Service: container-service
GitHub Login: @MGoedtel
Microsoft Alias: magoedte

@Sathya Rajesh Asokan
Thanks for your feedback! We will investigate and update as appropriate.

@MGoedtel
Could you please review this and update as appropriate.

@sathya-rajesh-asokan
Thanks for your feedback! I've assigned this issue to the author who will investigate and update as appropriate.

Thanks for your dedication to our documentation. Unfortunately, at this time we have been unable to review your issue in a timely manner and we sincerely apologize for the delayed response. The article has been updated unrelated to your feedback since creation of this issue and your issue has been resolved. We are closing this issue, but feel free to comment here as necessary. #please-close


Need to support dynamic libraries

so on gcc
dylib on apple clang
dll on msvc

Figure out how developer of build file specifies these. Is there anything to specify?
Figure out how consumer of library will include files. How does this affect cpplibroot?

Was a dup but thats fine there's support


[bug] OUT_DIR env var is not set, do you have a build script?

Describe the bug
fn main() {
  tauri::Builder::default()
      .invoke_handler(tauri::generate_handler![greet])
    .run(tauri::generate_context!())
    .expect("error while running tauri application");
}

Reproduction
No response
Expected behavior
No response
Full tauri info output
Macro expansion:
    compile_error!("OUT_DIR env var is not set, do you have a build script?" )
`generate_context` on docs.rs

Stack trace
No response
Additional context
No response

Well, do you have a build script? If you used create-tauri-app or tauri init it should have created a build.rs file in the src-tauri folder with this content:
fn main() {
  tauri_build::build();
}

same problem on win10, and I have the build script:  build.rs

Can you upload your app to github? Or try a fresh app create via create-tauri-app and compare that with your own app if it works?

I encountered the same behavior on win11 and tried to create a new app as @FabianLars suggested using the create-Tauri command. The error is also present in the new project. There is a build.rs file present in my projects. Besides the error message in my IDE, the build/app seems to function as expected. If you still need an example app: https://github.com/BitsAndDroids/FlightConnector-Rust

Besides the error message in my IDE, the build/app functions as expected.

So, are both of you encountering it just in an jetbrains ide? Because if so i'd assume they'll fix it in the next update. Or i really hope so because i'm not a fan of the PR tbh since it's really not our responsibility to set it (otherwise cargo build and rust-analyzer would fail too).

I'm trying another IDE, but I believe you found the culprit. I've just checked VSCode and the "bug" is gone. At least I've learned how to sign a pr from this. Will close the PR.

I'm not sure if this is even relevant, but in VSCode, the out_dir isn't set as well. But there isn't an error message displayed. So under the hood the behavior is the same but Jetbrains displayed the error while VSCode doesn't

Hi folks, we'll see what we can do on the RustRover's side: https://youtrack.jetbrains.com/issue/RUST-13238/Suspicious-macro-expansion-results-in-Tauri-apps

I meet the same problem in RustRover, I fix this by delete target  folder in the project root

Closing as the RustRover version that fixes this issue should be available now :)

same with rust analyser think deleting targets worked


Keep MSI article redirects in the CLI ToC.

:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

latest/docs-ref-conceptual/TOC.md
:white_check_mark:Succeeded

latest/docs-ref-conceptual/TOC.yml
:white_check_mark:Succeeded
View (azure-cli-latest)

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.


Be able to join dynamically on Groups

Thinking about how you could join across groups of the same measurement.
Joining across measurements is straight forward and documented.
How would you join two different groups from the same measurement? Do you even need to? I think so and I think you may want to do it dynamically as well.
Proposed solution:
.joinOnGroups(<group_selector or star>, ...)

A group select could just be { 'dim1' : 'value1', 'dim2': 'value2'} etc.
The number of arguments to joinObGroups indicates how many series are joined,
A * indicates all groups.
For example:
stream
  .from('errors')
  .groupBy('page', 'section')
  // do some work
  .joinOnGroups({'page' : 'home', 'section':'sidebar'}, *)
    .as('home', 'other')
  // do more work on the joined data.

This would join the home page sidebar section with every other page section group. Producing a new groups with tags like:
{
 home.page: home,
 home.section: sidebar,
 other.page: login,
 other.section: oauth,
}

Seems like manipulating tags like we do fields with dot separators is a simple solution.
Going to think on this some more but it feels like a good start.

This feature is pretty useful for my use case, I am currently creating alerts that looks on multiple instances and check their status relatively to other (for example: checking distribution of data/users between instances from the same type -> "all web servers should divide the users count euqaly").
So in my use case I can:
stream.from('users_count').groupBy('type').joinOnGroups(*).as('app1', 'app2')
// do some alert for (app1 / (app1.users_count + app2.users_count) / 2 (numbers of instance)) or app2.

Explanation of the calculation: to see if one instance has more than average user count and to alert on it.

@yosiat Thanks for the feedback. Glad to see its not just my crazy idea but that others have a use case in mind.


Show pinned workflow state for current branch

Think about if we want to scope the status displayed for pinned workflows, for example, to the currently checked out branch.

I definitely want this. Having a status bar icon that shows the state of a different branch is really not that helpful. What I want to see is always "did the thing I just pushed break the build". For the same reason, it would make sense for the polling to only run for a period after the push, and then stop polling once its got a result - as there's no point polling github every 30s when you already know the state of build for the last commit.

I definitely want this. Having a status bar icon that shows the state of a different branch is really not that helpful. What I want to see is always "did the thing I just pushed break the build". For the same reason, it would make sense for the polling to only run for a period after the push, and then stop polling once its got a result - as there's no point polling github every 30s when you already know the state of build for the last commit.


make types match

Current dependencies on/for this PR:

main

PR #31   👈

This stack of pull requests is managed by Graphite.

Merge activity

Jan 31, 4:45 PM EST America/New_York: @willseltzer merged this pull request with Graphite.


Atom v1.13.0 deprecation warnings

"Starting from Atom v1.13.0, the contents of atom-text-editor elements are no longer encapsulated within a shadow DOM boundary. This means you should stop using :host and ::shadow pseudo-selectors, and prepend all your syntax selectors with syntax--. ..."
Removed the one ":host" selector which was no longer needed, then just prepended all syntax classes with the required 'syntax--'.

Would it be possible to get this merged before Atom stops fixing the old styles automatically?

Sorry I've been away from my GH account for a while. Are you interested in getting commit access to this repo and continue maintaining it?

Sure, I'd be fine with that


In case of empty response, REST Client wait indefinitely

When response body has zero length (Content-Length :0) and response code is HTTP 200, REST Client wait indefinitely with wait dialog saying "Sending data...".
Same REST API works using curl:
curl -k -v -u xyz:123456 -k https://host:8443/api/blah/id=5

Authorization: Basic ABCDEFGHIJKL==
User-Agent: curl/7.19.7 (x86_64-redhat-linux-gnu) libcurl/7.19.7 NSS/<IP_ADDRESS> zlib/1.2.8 libidn/1.18 libssh2/1.4.2
Host: host:8443
Accept: /

< HTTP/1.1 200 OK
< Server: Apache-Coyote/1.1
< Pragma: No-cache
< Cache-Control: no-cache
< Expires: Thu, 01 Jan 1970 00:00:00 UTC
< Content-Type: application/json
< Content-Length: 0
I think problem is that REST Client require HTTP 204 status for zero length responses to correctly recognize empty body.
Anyway, there is no mention about it in HTTP spec AFAIK (correct me if I'm wrong).
Thanks.

same problem here

hi, check you are pasing the desired parameter or not.
because my problem was solved

The solutions to issues 93 and 124 work here. Remove the Content-Type header or set to some something other than application/json.


feat(linter): support vue generic component

closes #1938

Merge activity

Jan 11, 2:48 AM: @Boshen merged this pull request with Graphite.


Invoking long running commands on Linux, how to wait for completion?

Hi Guys,
I've got some long running commands (some are 10 seconds, some are 10 minutes) that I need to run on Linux and then I want to read the output and parse it.  I'm currently doing this which does not seem efficient:
$stream.WriteLine($cmd)
sleep 600 # sleep 10 minutes
$stream.read()
The problem is I have to guess how long to "sleep" for and that is error prone.
Is there any sort of cmdlet or method that will allow me to wait for some string (like the user prompt) and then automatically do a $stream.read()?  Something like a Invoke-SSHStreamExpect (which does not exist).
Pseudo code:
$cmd = "grep -R blah /etc" # This takes about 20 seconds to run
$result = $stream.expect($cmd, ']$')  # Run command and block until we see the last 2 characters of my user prompt.
Thanks,
Ben

This kind of works but it's not synchronous and i don't know when the command has finished:
Invoke-SSHStreamExpectAction -ShellStream $stream -Command 'grep -R blah /etc' -action 'date' -ExpectString ']$'

I think the stream object has several methods you can use including a expect one. Other than that I do not know of any other way.

On Oct 9, 2015, at 4:16 PM, Ben-Conrad<EMAIL_ADDRESS>wrote:
This kind of works but it's not synchronous and i don't know when the command has finished:
Invoke-SSHStreamExpectAction -ShellStream $stream -Command 'grep -R blah /etc' -action 'date' -ExpectString ']$'
—
Reply to this email directly or view it on GitHub https://github.com/darkoperator/Posh-SSH/issues/71#issuecomment-146976797.

Thanks for the tip.  I was reading the SSH.NET API Doc and got this code working as you describe:
$cmd = "grep -R blah /etc"
$stream.WriteLine($cmd)
$stream.Expect(']$')
Will block execution until the expect string is found, perfect!!!
$span = new-timespan -seconds 5
$cmd = "grep -R blah /etc"
$stream.WriteLine($cmd)
$stream.Expect('NeverMatchThisString',$span)
This will block waiting for the string match and then quit after $span if string not matched


Workflow service based tools

Ok @llam001 I am done syncing all of the changes to the branch. If you are good with everything, it is ready to merge.

@taweintraub Looks like the 11.0 folder is missing.

I think because I don’t have anything in there yet. The folder is in my repo…it just doesn’t have anything in it and it didn’t move across
Get Outlook for iOShttps://aka.ms/o0ukef

From: Lalaine Lam @.>
Sent: Monday, August 29, 2022 7:21:31 PM
To: Esri/workflowmanager-samples @.>
Cc: Tiffany Weintraub @.>; Mention @.>
Subject: Re: [Esri/workflowmanager-samples] Workflow service based tools (PR #13)
@taweintraub [github.com]https://urldefense.com/v3/__https://github.com/taweintraub__;!!CKZwjTOV!2d9Wmx8GUoPnDdRlLxfjCrt3cMu6ACFJcCwR60AkILqeWk1lxuKHGp2LHP3eW6BfjQ-R6remL9_5rEn6PFBw9Uo$ Looks like the 11.0 folder is missing.
[image] [user-images.githubusercontent.com]https://urldefense.com/v3/__https://user-images.githubusercontent.com/8961199/187321164-26daf8ad-a455-4e93-a601-9a3f2c31b87e.png__;!!CKZwjTOV!2d9Wmx8GUoPnDdRlLxfjCrt3cMu6ACFJcCwR60AkILqeWk1lxuKHGp2LHP3eW6BfjQ-R6remL9_5rEn6uauNCbs$
—
Reply to this email directly, view it on GitHub [github.com]https://urldefense.com/v3/__https://github.com/Esri/workflowmanager-samples/pull/13*issuecomment-1231001392__;Iw!!CKZwjTOV!2d9Wmx8GUoPnDdRlLxfjCrt3cMu6ACFJcCwR60AkILqeWk1lxuKHGp2LHP3eW6BfjQ-R6remL9_5rEn68Nb6tlI$, or unsubscribe [github.com]https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ARY6K34P5YB7PX434CIKW33V3VHYXANCNFSM57QELNXA__;!!CKZwjTOV!2d9Wmx8GUoPnDdRlLxfjCrt3cMu6ACFJcCwR60AkILqeWk1lxuKHGp2LHP3eW6BfjQ-R6remL9_5rEn6Uc2dp7Y$.
You are receiving this because you were mentioned.Message ID: @.***>

@taweintraub Looks good. Feel free to merge your changes.


Should we encourage the community to contribute to your fork?

Hi @regadas ,
This is not an issue, though I was not sure how else to start the conversation.
As you know, the original flink-on-k8s-operator is no longer maintained, I and several others have submitted PR to it but couldn't get them merged.
I'm happy to discover your fork seems active. Do you plan to continue working on this? Is it useful that I re-submit my PR here? Is it OK if I encourage other contributors to do the same?
Thanks a lot,
S

Hi @sv3ndk!

I'm happy to discover your fork seems active. Do you plan to continue working on this? Is it useful that I re-submit my PR here? Is it OK if I encourage other contributors to do the same?

Yes, feel free to contribute back and let others know! The plan is to keep this one moving forward.
Thanks for reaching out!

Hi @regadas,
Nice to hear that.
Do you (you as Spotify) have any plan to increase bus factor of this project ?

Hey, we have the same problem with this repo (started in april when I opened https://github.com/GoogleCloudPlatform/flink-on-k8s-operator/issues/441)
tried to move things from the slack channel but no luck there.
We have our own private fork with many changes (major concurrency bug fix, savepoint taking imrovements, k8s 1.20+ support, etc..)
Do you guys want to talk maybe about this project and its open source future so we can join our efforts?

? @sv3ndk @regadas

@shashken Yes feel free to send PR's! some things might have been already addressed but yeah every contrib is very much welcomed!


How to prompt Llama2-13b-chat to generate false answers?

In Section 4.1, it mentions that there are 3 ways to obtain stimulus for truthfulness.

(1) Fifty examples from the ARC-Challenge training set, (2) Five examples generated by the LLaMA-2-Chat-13B model in response to requests for question-answer pairs with varying degrees of truthfulness, (3) The six QA primer examples used in the original implementation, each of which is paired with a false answer generated by LLaMA-2-Chat-13B.

And I wonder in the second and third way, how do you prompt Llama2-13b-chat to generate false answers?

We've tried different prompts which mostly gave similar results.
An example prompt for (2) is
'Generate 5 world questions and give 2 answers to each, one being truthful and the other being untruthful.'
An example prompt for (3) is
f'[INST] Consider the following question: {q}\n'
f'The correct answer is: {a}\n'
'Please generate an incorrect answer. Make sure the answer is plausible. There is no need to give an explanation.'
'[\INST]\n'
f'Question: {q}\nCorrect answer: {a}\nIncorrect answer: '


Publish DelegateTask/DelegateTask/HistoryEvent as Spring Event

like camunda-bpm-reactor this allows execution of task/execution listeners via eventing without changing the models.
see https://app.camunda.com/jira/browse/CAM-8965
this PR is not finished, I just wanted to publish early so we can discuss if this is worth to carry on

Please review this PR. @sdorokhova

Reassigning this, as @sdorokhova is currently not working on this project.

Hi @zambrovski, Jan,
Thanks for the contribution.
I have a look into it and provide you some feedback later this week.
Best regards,
Yana

Hi Simon,
I tried to configure an execution listener, and it worked like a charm.
In general, the feedback is positive, it is a nice feature, and it will be great to include it.
Here are some adjustments that will be nice to consider before I merge the PR:

Could you please add the license header to the files as described in Contribution guide.
Could you please adjust the test methods name to be camel case formatted
I was not able to disable (camunda.bpm.eventing.execution: false) the publishing of the events via the properties file. I guess that it is not respecting the properties file configuration. Could you please have a look at it?
It would be great if you rise a pull request for documenting the feature. Without proper documentation, the users most likely won't benefit from it.

Best regards,
Yana

Hi Yana,
thank you for the feedback. I resolved the issues above and provided a docs pull request in the
docs repo: https://github.com/camunda/camunda-docs-manual/pull/218
Kind regards
Simon

Hi Simon,
That's great, I will have a look at it.
Best regards,
Yana

Hi Simon,
Thanks for the quick response, everything looks good.
I squashed the commits and merged it to the master (f1c0998).
I forgot to mention in my previous reply that we need to implement the #parseProcess() as well because, in terms of configuring an ExectutionListener on a process level, it will be nice to have. So I adjusted that by myself.
You asked about an intermediate release before the 7.11. We can release an alpha version as an intermediate release containing this feature. I hope that it will suit your needs.
Best regards,
Yana

Hereby I am closing the PR.
In case of any further questions, feel free to reopen it.
Once again great feature 🎉
Thanks,
Yana


Bug fix on GalleryContainer.js.

Overview of Changes:

Fixed bug on GalleryContainer.js. Added props as a parameter on line 11.

Why was this done?

App was breaking after a fresh pull from main. This was necessary to get it working again.

Props on the props.


UserData.txt is not being created and populate. Scan will not run in background after choosing "Y"

Two issues with the latest release:
1.) UserData.TXT is not being created. I had to manually create a blank file. THis elimiated the name, email questions and allowed me to run my scan. This was an issue several releases back as well.
2.) WHen running the scan, if I select "Y" to run it in the background, the scan runs in the foreground, so Chrome opens and I can see it scanning.

Hi @jcrain23,
Thanks for reporting the issue.  I have made the fix in https://github.com/GovTechSG/purple-a11y/pull/269.  A release will then be created soon this week.
Cheers!


Extracting on non-byte boundaries causes infinite loop

I'm noticing that packet_of_bytes spins out if asked to extract a VBit of width 7. The Cstruct API only allows byte-aligned operations on the buffer so this may be a pain to fix.

This is fine. Let's say that we only support byte-aligned headers.
-N
On Mon, May 11, 2020 at 12:01 AM Ryan Doenges<EMAIL_ADDRESS>wrote:

I'm noticing that packet_of_bytes spins out if asked to extract a VBit of
width 7. The Cstruct API only allows byte-aligned operations on the buffer
so this may be a pain to fix.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/cornell-netlab/petr4/issues/174, or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAOCFUTKIMVW3Z7S7KZMTFLRQ52DDANCNFSM4M5RXMNQ
.

:+1: Just added a precondition check in my branch so it crashes instead of spinning out.

Noticed that p4c will reject a header if the width of the entire header is not byte aligned, but it doesn't care about fields. See this test.

OK, let's try to get rid of the field alignment requirement

Assuming that "extraction" is a typo and the intent was "emission", i think i came up with a fix where i concatenate all of the fields of a header into a single bitstring before converting them to a Cstruct. I couldn't find the test linked above but this change did cause about 5 tests to pass without introducing any regressions. Should this be enough to mark this issue resolved?


Fix server error due to missing environment variable.

Description
Use process.cwd() instead of relying on the environment variable which might be unset
Fixes # (issue)
Type of change

[x] Bug fix (non-breaking change which fixes an issue)

Screenshots
Checklist:

[ ] My code follows the style guidelines of this project
[ ] I have performed a self-review of my own code
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation

@kelvinkipruto  @kilemensi
Seems like process.env.PWD is returning undefined in the production server which is causing the App to crash.

@kelvinkipruto @kilemensi Seems like process.env.PWD is returning undefined in the production server which is causing the App to crash.

Yes @esirK, I think it was a typo. I don't think there is such thing as process.env.PWD. It's always supposed to be process.cwd(). @kelvinkipruto correct me if I'm wrong though.


Added basic functionality to SimpleCallManager, so that ByuCallback d…

…oesn't get rid of a call without calling a callback method

This change is

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable


Container networking

Overview
This issue tracks the design and implementation status of container networking, in its various forms.

Host-to-container (h2c)
Container-to-host (c2h)
Container-to-container (c2c)

Use cases
Expose a service to the host
SSH agent forwardingConnect to docker engineConnect to a service only reachable from the host
Run a test databaseRun e2e integration testsRun sidecar services

Status
❌ not implemented
🏗 in development
❌ not implemented

Notes
Will only be available for long-running containerized services, not short-lived exec
Unix sockets: ✅ implementedTCP: 🏗 API discussionUDP: 🏗 API discussion
Use bass c2c networking as a reference

Design proposals
Container networking is a vast feature area. There are several ongoing proposals to design certain parts of it. Some of these proposals are compatible, while others are not.

Proposal
c2h (tcp/udp)
h2c (tcp/udp)
c2c
c2h (unix)
h2c (unix)
status

Mount unix socket into exec
❌
❌
❌
✅
❌
Implemented

forwarding host TCP/UDP addresses to container
✅
❌
❌
❌
❌
In Discussion

inbound and outbound tunnels
✅
✅
❌
❌
❌
Withdrawn

bass-style c2c networking
❌
❌
✅
❌
❌
In discussion

“bass-plus” container networking
✅
✅
✅
❌
❌
In discussion

Submitting a proposal

Open a draft PR with graphql schema changes and, ideally, skeleton documentation
Post a link to the PR in the comments to this issue. Mention which areas are covered by the proposal (c2h, h2c, c2c)

Discussing a proposal

Comment on the relevant PR

Research material

SSH port forwarding (ssh -L and ssh -R)
Reverse proxy configurations (nginx, apache, varnish, envoy)
Kubernetes and Docker networking terminology
Bass-lang networking

Are we considering the container-to-host case (i.e. I want to run a dev webserver in dagger and access it from my browser) to be the same and host-to-container? In theory they are technically different (who does the connect, who does the listen), though if you do a reverse proxy thing locally then the can become the same.

Also, should probably clarify whether "container" means "exec" or "service" or both. My starting point is that we should not support serving sockets from execs and only serve them from services; execs are only able to consume them. But there's a lot to unpack and think about there so I'm far from adamant, just my first inclination.

My starting point is that we should not support serving sockets from execs and only serve them from services;

I agree -- had some comments about that in https://github.com/dagger/dagger/pull/3691#issuecomment-1311145961
IF that's the case, then we don't necessarily need to mirror all calls for bind/connect.
My straw man proposal for container-to-service was to mirror host:
c.Container().WithTCPSocket(8080, c.Host().TCPSocket(8080))
c.Container().WithTCPSocket(8080, myService.TCPSocket(8080))

Regarding host-to-service (IF it's always a service), then we can have some service specific instructions:
// 1) At service creation time
c.Service().Create(<args from the future>, dagger.ServiceCreateOpts{
  Expose: [some port bindings]
})

// 2) Dynamic exposure
// ??? Unsure how it'd play out ??
// Probably a bit too dynamic (once the service is running it's too late to mess with the shim, without some serious effort)
myService.TCPSocket(8080).ExposeTCP("localhost:8080")

// 3) CLI command
$ dagger service proxy <my service>:8080:8080

Regarding host-to-service (IF it's always a service), then we can have some service specific instructions:

I like option 1, personally. Keeps it in code, keeps it at service "boot". The only suggestion I have is that we clarify that it is being exposed on the host, not just within the Buildkit network/context. I'm not quite sure how to do that yet, but just Expose feels a bit unclear to my eyes.
Now that I've written that down, I'm wondering if it's possible have an API like this:
type Host {
  bind|expose(service: Service!): ??
}

That would make pure GraphQL service queries a bit harder/uglier, but I like the clarity that would provide for the SDKs.

+1 to starting with a distinction between services and containers for the sake of conversation.
Depending on how this goes, it might be the case that the service's container needs to be aware that it's being used as a service - via the underlying LLB we generate and/or the responsibilities of the shim - but we can cross that bridge when we get to it.
I put a bunch of details around how Bass's c2c/service networking works here: https://github.com/dagger/dagger/issues/3850#issuecomment-1329899415
Here are some questions based on that:
How do we want the traffic to flow in the following snippet?
c.Container().WithTCPSocket(8080, myService.TCPSocket(8080))

Typically a client container will need to be given the address to reach as a flag/arg/env/etc. With this scheme would they just use <IP_ADDRESS>:8080?
Does that imply we have something proxying traffic between them? Are we OK with the performance hit there? The answer might simply be yes, and maybe it's a minor performance hit anyhow, but it's worth trying to think about any performance-critical use cases here. If we only care about dev/test this is probably fine. It's hard to imagine someone running e.g. a production database with Dagger since it's not really built for persistence.
Another option is to use direct c2c addressing with a bridge network (+ DNS to prevent ephemeral container IPs from busting the cache) but then we'd need to figure out how the user configures the address for the client container. We can also use DNS to avoid cache busting while keeping the proxy to solve that.
Will we implement health/liveness/readiness checks?
My take on this is the minimum viable health check is to just wait for a port to be listening, and that we should just do that by default since it's really annoying to have to manually poll for simple cases.
I don't have a strong opinion on continuous health monitoring beyond that.

@jlongtine Good question - I think we likely won't ever need to call 'expose' for container-to-container (within Buildkit network). So I think we can get away with having Expose live on the service and have it always mean "expose on the host" which would draw parallels to e.g. Container.Export, Directory.Export which also affect the the host.

I'm unable to discuss these examples constructively, because WithTCPSocket does not clarify whether I'm connecting or binding.

@jlongtine Good question - I think we likely won't ever need to call 'expose' for container-to-container (within Buildkit network). So I think we can get away with having Expose live on the service and have it always mean "expose on the host" which would draw parallels to e.g. Container.Export, Directory.Export which also affect the the host.

Yeah, this makes sense, @vito. I'm on board.

Yesterday we had a 1-1 chat with @vito that melted my brain. After sleeping on it, I am updating this issue with:

A rewrite of the context and summary table. Hopefully it's more useful now
A list of open questions
A proposal for terminology and API design. To quote @vito: "i think there's potentially something better still, but it's a good strawman"

Please take a look and share your thoughts.

Addressing a few comments and questions above:

@sipsma:
Are we considering the container-to-host case (i.e. I want to run a dev webserver in dagger and access it from my browser) to be the same as host-to-container? In theory they are technically different (who does the connect, who does the listen), though if you do a reverse proxy thing locally then they can become the same.

👍 You are right, they are different and my framing was awkward. The root cause of the awkwardness was the use of the term "socket" which does actually apply to both (a socket is something you either connect or bind. then bytes are forwarded in both directions). Removing the term "socket" from feature name and API design allowed me to reconcile 1) non-confusing terminology for the thing being forwarded, and 2) non-confusing terminology for the "direction" of things.

@sipsma:
Also, should probably clarify whether "container" means "exec" or "service" or both. My starting point is that we should not support serving sockets from execs and only serve them from services; execs are only able to consume them. But there's a lot to unpack and think about there so I'm far from adamant, just my first inclination.
@aluzzardi :
I agree

👍 I agree also, clarified this in the issue.
For high-level discussion, it's still useful to use c2h/h2c/c2c (3 terms), instead of c2h/s2h/h2s/c2s/s2s (5 terms) :) But that's just terminology, we're all on the same page on design and implementation.

@aluzzardi @vito @jlongtine
(on the concept of "expose" for h2c)

🙋🏻♂It makes sense in theory, since we have "directory export" and "container export" which follow the same pattern.
However, I think we should not use this particular pattern here, because it is not intuitive whether the port is actually bound in the host, or simply available for ulterior service discovery from either the host or a container.
In Docker we had terminology for both: you could "expose" a port (available for service discovery), or "publish" it (bound to the host). This was generally confusing to users.
I propose a variation of this pattern that still controls the tunneling from Service (rather than Host), but is more explicit about what's going on. withInboundTunnel clearly means "create a tunnel that will forward inbound traffic".

With the implementation of socket forwarding, users have started to spawn local registries to their host docker engine, using them as a long-runner engine: https://github.com/dagger/dagger/issues/4089
By relying on Docker for most long-running tasks:

the Host-to-container (h2c) is not as urgent as Container-to-host (c2h). Same for the Container-to-container (c2c), which could be workarounded by users if we had access to c2h primitives
healthchecks are quite easy to implement natively and might alternatively be tackled using extensions:

// query localhost:5000/v2/ until it returns 200 with a 20 seconds timeout
func waitForRegistry() error {
	ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
	defer cancel()

	req, err := http.NewRequestWithContext(ctx, "GET", "http://localhost:5000/v2/", nil)
	if err != nil {
		return err
	}

	client := http.Client{}
	for {
		resp, err := client.Do(req)
		if err != nil {
			// retry
			time.Sleep(1 * time.Second)
			continue
		}

		if resp.StatusCode == http.StatusOK {
			return nil
		}
	}
}

Current blockers
I am currently relying on the WithMountedSockets API for #4038.
However, I am blocked by the fact that I cannot export an image (using our core API) to my private registry if it is not part of the dagger-engine network. I currently have 2 options:

create a registry that is part of the dagger-engine network
push my image using the docker CLI

I feel that the socket implementation tackles well the SSH forwarding issues, but the Container-to-host begins to feel needed for more complex use-cases

Typically a client container will need to be given the address to reach as a flag/arg/env/etc. With this scheme would they just use <IP_ADDRESS>:8080?
Does that imply we have something proxying traffic between them? Are we OK with the performance hit there?

I remember reading the design in bass around services and it made a lot of sense. Do you have something in mind for this API that could avoid the proxying? We're now free to customize buildkit however we want

Will we implement health/liveness/readiness checks?
My take on this is the minimum viable health check is to just wait for a port to be listening, and that we should just do that by default since it's really annoying to have to manually poll for simple cases.

I think it would make sense to have a minimum viable health check. Otherwise, without the user implementing something, it's likely to fail every single time (e.g. create service mysql, exec query => instant failure).
This might require configuration -- we should at least make sure there's a "spot" in the API where that config could eventually fit.

API design for container-to-host

  withOutboundTunnel(protocol: TunnelProtocol=TCP, backend: String!, frontend: String): Container!

nit: This is inconsistent with the pattern of going through Host(), e.g. WithDirectory("/foo", c.Host().Directory(".")).
(not rhetorical) so why not c.WithHostDirectory() and getting rid of c.Host().Directory()?
Another factor is container-to-service. The reason I found dagger.Socket compelling is it gave a layer of indirection (disregard the word "socket") and a nice symmetry with directories:
// FS
ctr.WithDirectory("/foo", dg.Host().Directory(".")) // c-to-h FS
ctr.WithDirectory("/bar", anotherCtr.Directory("/bar")) // c-to-c FS

// Network
ctr.WithSomeNetwork(something, dg.Host().SomeNetwork(8080)) // c-to-h NET
ctr.WithSomeNetwork(something, aService.SomeNetwork(8080)) // c-to-s NET

I remember reading the design in bass around services and it made a lot of sense. Do you have something in mind for this API that could avoid the proxying? We're now free to customize buildkit however we want

Here's a mockup, I think this as close as I can get it to Bass's setup:
func TestServices(t *testing.T) {
	c, ctx := connect(t)
	defer c.Close()

	www := c.Directory().WithNewFile("index.html", "Hello, world!")

	// NB: for now I'm just making this a Container, ignoring the Container vs
	// Service argument to stay focused
	srv := c.Container().
		From("python").
		WithMountedDirectory("/srv/www", www).
		WithWorkdir("/srv/www").
		WithExec([]string{"python", "-m", "http.server"}).
		// optional: not a fundamental requirement, but I suspect named ports will
		// be easier to work with + refactor across abstraction boundaries. it also
		// tells Dagger what ports to health check.
		WithExposedPort("http", 8080)

	client := c.Container().
		From("alpine").
		// similar to WithSecretVariable. introduces a dependency between
		// client<->server and configures its address as an env var.
		WithServiceVariable("URL", srv.Address("http", "http://$host:$port")).
		WithExec([]string{"sh", "-c", "wget -O- $URL"})

	// running the client will:
	// 1. start the server as a buildkit solve(). we use a solve() so that
	//    concurrent runs are deduped. buildkit automatically does this + proxies
	//    progress to all clients.
	// 2. poll for the service's exposed ports to be bound.
	// 3. render the $URL ServiceVariable using the server's DNS address and port.
	//    note that this API would also support tunneling if we need to; just
	//    replace $host with '<IP_ADDRESS>' and $port with whatever ephemeral port
	//    we use in the container.
	// 4. run the client with $URL
	// 5. detach the server (cancel() ctx). note that we detach instead of
	//    stopping; this way any concurrent instances will continue until all
	//    clients detach.
	out, err := client.Stdout(ctx)
	require.NoError(t, err)
	require.Equal(t, "Hello, world!", out)
}

This is roughly equivalent to the following Bass script:
(defn http-server [index]
  (from (linux/python)
    (-> ($ python -m http.server)
        (with-mount (mkfile ./index.html index) ./index.html)
        (with-port :http 8000))))

(defn main []
  (let [srv (http-server "Hello, world!")]
    (-> ($ wget -O- (addr srv :http "http://$host:$port"))
        (with-image (linux/alpine))
        (read :raw))))

Some notes:

We have to use an env var and sh -c to pass a service address as a CLI arg, since we can't intermingle a complex address type with the regular string args.
DNS addresses would not be user-configurable, to avoid collisions. Instead they would be a content-addressed hash. For Bass I use base32 + xxh3 to get short addresses like GRLO5DSIPLJPQ.
This involves the use of a template string for $host and $port, which is a little interesting. Open to other ideas there.
This is all only concerned with IP addressing. I'm not sure to what extent we care about Unix sockets here.

@vito in your c2c design, are there constraints in how we implement IP addressing and routing? For example can the IP address be different between runs of otherwise identical services? Could the operator eventually configure IP ranges to use, or allocate them themselves via a hook, or would that break the design.

nit: This is inconsistent with the pattern of going through Host(), e.g. WithDirectory("/foo", c.Host().Directory(".")).

But it's consistent with Container.Export and Directory.Export :)

(not rhetorical) so why not c.WithHostDirectory() and getting rid of c.Host().Directory()?

I don't have a good answer. Empirically, this part of the current API has been fine in practice. I think it's because Host().Directory() is a great starting point for many different kinds of chains. Whereas a Socket type or equivalent isn't really chainable: there's only one place you can use it. So maybe the difference is in the level of composition we need?

@vito in your c2c design, are there constraints in how we implement IP addressing and routing, and implementation of backing network interfaces? For example can the IP address be different between runs of otherwise identical services? Could the operator eventually configure IP ranges to use, or allocate them themselves via a hook, or would that break the design.

Answering myself with details from this other comment :

Bass containers reach services by addressing them over a bridge network, using DNS to resolve to container hostnames to IPs at runtime.
DNS was my solution for not busting the cache. The key point is to avoid having ephemeral container IPs ever enter into LLB. I got DNS to work by configuring Buildkit to use CNI and installing dnsmasq and the dnsname plugin:

DNS addresses would not be user-configurable, to avoid collisions. Instead they would be a content-addressed hash. For Bass I use base32 + xxh3 to get short addresses like GRLO5DSIPLJPQ.

@vito What's the input to the hash you use in bass? The LLB of the service being depended on? If so, then I'm wondering how we'd deal with sources where the LLB is stable but the actual contents can change (e.g. image:latest or local dirs)

@vito What's the input to the hash you use in bass? The LLB of the service being depended on? If so, then I'm wondering how we'd deal with sources where the LLB is stable but the actual contents can change (e.g. image:latest or local dirs)

Bass uses the hash of the thunk, so it's a hash of the LLB "recipe" rather than the actual LLB. Thunks are fully specified - they pin image digests and don't use any other Git/HTTP LLB sources. Just images and commands.
So this is a great point. I think it would be fine to use a hash of the LLB even with unresolved references since they should be stable within that session, as long as the single session is used for running both the service and the client. This definitely relates to #3613 especially if that's not actually possible for some reason (similar to the current Container.Export limitation).

Adding my +1 to this feature, I would like to use this for running pre migration validations on databases and I need to spin up a MySQL DB and another container that communicates with it

I've got a few use cases for this.

as @yonidavidson said, we have a pipeline that needs to run validations and migrations against a database
we need to access IPs that are only accessible from the host networking

Both locally and in Github actions we are able to reach these systems through tailscale.

@vito with #4505 merged, can you update the issue description? It looks like it wasn't updated since Dec 13th, so I want to make sure that it reflects the work that still needs to be done. Thanks!


Fix incorrect style of nested links in BitNav

This resolves #592 by fixing the incorrect style of
nested links in BitNav component

Since the BitNav component went under a heavy refactor and improvement, this PR is not valid anymore.
so I'm closing it.


Get details of merge request

As NHS admin I want to see details of merge request I reviewing in order to make decision on it
We need to implement WS

[ ] logic / schema
[ ] implement
[ ] test

there is a problem with checking role, I still get merge requests list/id without required role NHS REVIEWER in case I have required scope

I've perform this check with client_id field, should do with consumer_id?

that's neither about cient_id nor consumer_id. We should check user's roles within client


ISPN-6939 RocksDB Cache Store

https://issues.jboss.org/browse/ISPN-6939
There is a lot here, but mostly because of the churn of replacing the LevelDB store. The actual RocksDB cache store is a straightforward port.

@ryanemerson I have addressed all concerns

@tristantarrant There are several related test failures that need addressing

Should have fixed all pending issues.

@tristantarrant There still appears to be related test failures

Ok @ryanemerson all remaining failures are unrelated.

Integrated 4e3e495a309f5c1b09fff7da94c5eada0627aad1 and 882b2a82e3ac01939603319b44f55923483751a5


Output better XAML errors

Avalonia XAML error messages are sometimes less than ideal. This can be a big issue particularly for new users as the error messages tend to confuse more than help. Here's a few problems with them:

They duplicate the line/column numbers
They include the csproj which isn't (ever?) useful
The type information is noisy (always includes assembly and full namespace)
Types/members not quoted
They don't always give the correct column info (the open tag is marked as having the error when in reality the error occurs on e.g. a property)
The error message isn't always clear
They include irrelevant information
Setting value to a readonly property should show a better error https://github.com/AvaloniaUI/Avalonia/issues/12218

Some examples follow. This list isn't exhaustive but shows some of the more common errors:
Missing property
Invalid XAML
<Button Foo="Bar"/>

Avalonia
Unable to resolve suitable regular or attached property Foo on type Avalonia.Controls:Avalonia.Controls.Button (line 5 position 6) Line 5, position 6. [D:\projects\AvaloniaUI\Avalonia\samples\ControlCatalog\ControlCatalog.csproj]

WPF
The property 'Foo' does not exist in XML namespace 'http://schemas.microsoft.com/winfx/2006/xaml/presentation'. Line 4 Position 13.

Evaluation

Duplicate line/column numbers
csproj path is included
Types/members not quoted
This isn't attached property format so no need to mention "attached property"
Avalonia.Controls:Avalonia.Controls.Button is overkill: Button would suffice
The WPF error isn't much better to be honest

Better Error Message
Unable to resolve property 'Foo' on type 'Button'. Line 5, position 6.

Missing Event Handler
Invalid XAML:
<Button Click="Foo"/>

Avalonia:
Unable to find suitable setter or adder for property Click of type Avalonia.Controls:Avalonia.Controls.Button for argument netstandard:System.String, available setter parameter lists are: [D:\projects\AvaloniaUI\Avalonia\samples\ControlCatalog\ControlCatalog.csproj]
System.EventHandler`1<Avalonia.Interactivity.RoutedEventArgs> (line 5 position 13) Line 5, position 13. [D:\projects\AvaloniaUI\Avalonia\samples\ControlCatalog\ControlCatalog.csproj]

WPF:
'MainWindow' does not contain a definition for 'Foo' and no accessible extension method 'Foo' accepting a first argument of type 'MainWindow' could be found (are you missing a using directive or an assembly reference?)

Evaluation

netstandard:System.String is overkill
System.EventHandler``1<Avalonia.Interactivity.RoutedEventArgs> isn't particularly useful for new users
Appears as two error messages
Says "property Click" but Click is an event
Again, WPF isn't much better though at least it's less verbose

Better Error Message
Unable to find suitable handler for event 'Button.Click'. Valid handler signature is 'void Button_Click(object sender, RoutedEventArgs e)'.
Invalid value
Invalid XAML:
<Button HorizontalAlignment="Foo"/>

Avalonia:
Unable to find suitable setter or adder for property HorizontalAlignment of type Avalonia.Layout:Avalonia.Layout.Layoutable for argument netstandard:System.String, available setter parameter lists are: [D:\projects\AvaloniaUI\Avalonia\samples\ControlCatalog\ControlCatalog.csproj]
Avalonia.UnsetValueType [D:\projects\AvaloniaUI\Avalonia\samples\ControlCatalog\ControlCatalog.csproj]
Avalonia.Data.IBinding [D:\projects\AvaloniaUI\Avalonia\samples\ControlCatalog\ControlCatalog.csproj]
Avalonia.Layout.HorizontalAlignment (line 5 position 13) Line 5, position 13. [D:\projects\AvaloniaUI\Avalonia\samples\ControlCatalog\ControlCatalog.csproj]

WPF:
Requested value 'Foo' was not found.

Evaluation

Appears as three error messages
The info about UnsetValueType/IBinding isn't very useful as this obviously isn't a binding

Better Error Message
'Foo' is not a valid value for 'HorizontalAlignment'.

Another from gitter: https://gitter.im/AvaloniaUI/Avalonia?at=5f32b56eb103e45c830e2c2d

Maybe add warning support as well? For example, Obsolete

"Unable to find the parent scope for AvaloniaProperty lookup" error can add a hint about where TemplateBinding can be used. And recommendation to use Bindings instead:
https://github.com/AvaloniaUI/Avalonia/issues/15572

This too: https://github.com/AvaloniaUI/Avalonia/issues/16144

This too https://github.com/AvaloniaUI/Avalonia/discussions/16580#discussioncomment-10246589

https://github.com/AvaloniaUI/Avalonia/issues/17562


Add support for linux ARM64 in Ibazel

Describe the bug
A clear and concise description of what the bug is.
We are trying to install and use ibazel on linux-arm64 environment. But we are receiving error. By looking at code, we found that in node_modules @bazel/ibazel/bin folder does not contain linux_arm64 package and thus we code is throwing error.
Reproduction instructions
Install ibazel on linux-arm64 and receiving below error:
WARNING: no ibazel version found in your node_modules.
        We recommend installing a devDependency on ibazel so you use the same
        version as other engineers on this project.
        Using the globally installed version at /usr/local/share/npm-global/lib/node_modules/@bazel/ibazel
node:events:504
      throw er; // Unhandled 'error' event
      ^

Error: spawn /usr/local/share/npm-global/lib/node_modules/@bazel/ibazel/bin/linux_arm64/ibazel ENOENT

If you'd like to extend on the work of mistic in https://github.com/bazelbuild/bazel-watcher/pull/496, I'd be happy to merge a PR to add this to be built as a part of the release process, but I don't have an arm64 linux box to work off of so I'm not going to be able to do that.

@achew22 do you have raspberry pi 4? If yes then it is a ARM processor and you can install official ubuntu OS into it. Check here https://ubuntu.com/download/raspberry-pi
Let me know if any efforts are required from my side.
Thanks

Sorry to say, I do not have a raspberry pi 4. I think the PR I linked is enough to connect the dots if you want to try it out, otherwise this will have to wait until someone has something that calls for it which I don't see coming soon from me.


Introduce este-firebase-redux

Separate project (este-firebase-redux) now exists with exactly the same code but as an external NPM module.
We should pull it in.

Sure, thank you!

I'm working on it in a separate branch.

Feel free to reopen once everything will be ready.
(I told you open source is time consuming ;)


fix-request OIDCCacheType comments

Hi!
I will sugest it, Beacuse I'm going to make a misteke. 😄

the extra brackets that you wish to see removed indicate the optionality of the Redis value: only when compiled in with Redis support, this value is available


2nd Iter: Ensure that hostnames without dots are excluded.

This patch fixes #2347.
This patch touches #2391.
Indeed, my previous patch was missing domains with dashes (-).

Converting to draft as I caught more errors.

Hey Steve @StevenBlack! Hope you are doing well.
I have a question because I caught some "discrepancies" while trying to implement this right.
The question: What is your position on IPs?
I'm asking because, on one side, we have:  https://github.com/StevenBlack/hosts/blob/86956052240f198dad3d3a9b74b9c3a4e0f71340/testUpdateHostsFile.py#L841-L853
which says: we don't want any "raw" IP.
On the other side, we have:
https://github.com/StevenBlack/hosts/blob/86956052240f198dad3d3a9b74b9c3a4e0f71340/testUpdateHostsFile.py#L892-L905
which says: if an IP is given as part of a host entry, keep it.
Finally, on one other side, we have: https://github.com/StevenBlack/hosts/blob/86956052240f198dad3d3a9b74b9c3a4e0f71340/updateHostsFile.py#L1091-L1092
which says: just ignore IPv6.
That's okay, but we both know, it doesn't make sense at all to have 2 IPv4s in a host's file entry.
The reason I'm asking is while analyzing the outputs of the coming commits, I discovered, that we have to change the way we catch domains. Indeed, the only records that were not being scrapped correctly, were those where the Punycode of the TLD has a number and dashes in it. That means that to stay as correct as possible, we have to relax the regex - somehow.
While I did so locally, I now have the problem that IPs pass the regex which was meant for domains. So now I have to change not only the regex but some parts of the code.
Therefore, we need to decide what we want.
.... To keep (IPv4s) or not to keep ? That is the question! 😅
Have a nice day/night!
Nissar

Thank you Nissar @funilrys I'll look into the history of that test code now. I agree it doesn't seem to make sense.

Nissar @funilrys I'm seeing a failing test here.  I'm still puzzled over the test — what is it actually testing? — it appears to be a test that combines the --ip flag with an invalid domain (an ip-address) for input?
Here's what I'm seeing
 python3 testUpdateHostsFile.py
...........................................F.........................................................................
======================================================================
FAIL: test_no_match (__main__.TestNormalizeRule)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/Steve/Dropbox/dev/hosts/testUpdateHostsFile.py", line 850, in test_no_match
    self.assertEqual(normalize_rule(rule, **kwargs), (None, None))
AssertionError: Tuples differ: ('<IP_ADDRESS>', '<IP_ADDRESS> <IP_ADDRESS>\n') != (None, None)

First differing element 0:
'<IP_ADDRESS>'
None

- ('<IP_ADDRESS>', '<IP_ADDRESS> <IP_ADDRESS>\n')
+ (None, None)

----------------------------------------------------------------------
Ran 117 tests in 0.091s

FAILED (failures=1)

ALSO check out some of the warnings above ⬆ that read like this:

It feels like we are thisclose 🤏🏻 but still not right.
I've taken several stabs at this regex in the past.  I'm currently thinking, regex may not be the best tool anymore at this juncture.

So, as I understand, <IP_ADDRESS> gets matched by ^\s*((?:[\w\-\.]+\.)+[a-zA-Z0-9\-]+)(.*), which checks for raw domains not preceded by a destination. It boils down to the question of whether we want to allow IP addresses in the host field, as mentioned by @funilrys
In my opinion, I'm inclined to say no, since mapping IP addresses to other IP addresses is not something the hosts file should be doing.
As for the inefficient regex, I do believe many parsing issues are prevented by not using regexes, but a couple potential optimizations are:
^\s*(\d{1,3}\.){3}\d{1,3}\s+((?:[\w\-\.]+\.)+[a-zA-Z0-9\-]+)(.*) -> ^\s*\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\s+([\w\-\.]+\.[a-zA-Z0-9\-]+)(.*)
Writing out each octet of the IP address seems to reduce the number of steps, at the cost of verbosity. I also removed the first match group (\d{1,3}\.){3}, since it didn't seem to be used? I have a strong hunch that (?:[\w\-\.]+\.)+ is what could potentially be causing the exponential backtracking issue. At the very least, removing the non-capturing group seems to reduce the number of steps. My rationale for safely removing it is that as long as the sequence ends with ., preceeding groups should be matched by [\w\-\.]+. The effect should be the same as explicitly checking for . at the end of each group.
^\s*((?:[\w\-\.]+\.)+[a-zA-Z0-9\-]+)(.*) -> ^\s*([\w\-\.]+\.[a-zA-Z0-9\-]+)(.*)
follows the same ideas. With the <IP_ADDRESS> test removed, the others fail because the (.*) at the end matches the space character and everything after it. Otherwise, might be able to fix this with a lookahead assertion? Assertions could also be used to block IP addresses, if/when relevant.
The output of the hosts file is unchanged, though, save for something that actually appears to be fixed. <IP_ADDRESS> pgl.example #0101`` -> <IP_ADDRESS> pgl.example0101`

Hey @StevenBlack ,
I implemented a "regexless" generic solution that should solve all our problems.
Please look at my last commit and proceed with the tests.
My last commit assumes we want to get rid of IPs as part of the rule. Therefore, I will need a decision regarding the failing test rules.
Stay safe and healthy!
Nissar

Following Steve's @StevenBlack comment (CF: https://github.com/funilrys/hosts/commit/b3f93f1528361910abb09e06be0f2ecdc5aeb665#commitcomment-126334510), I submitted the final changes.
Please review.
Stay safe & healthy!

Nissar THANK YOU SO MUCH ❤
Tests work great, updating works great.  This is awesome!
MERGING!

Thank you for your input as well @buzzingwires that was very valuable too!

Nissar @funilrys something I just noticed: domains are no longer all converted to lowercase.
For example, in the base hosts file line 219361: PopCash.com
Also the list jumped by 10k domains with this new version, which is about 8,000 more than I expected.

#2347

#2391


Relax others Qiskit version dependencies

What is the expected behavior?
I'm all about depending on specific versions for third-party depedencies, in general it's a good "damage control" strategy, but I'd definitely relax our own Qiskit version depedencies, because we control them and we have better response times in case of an event.
Also, that will give different teams more freedom to patch their components without any further interaction with the rest of the components.
The teams behind the components are the responsible of keeping them in good shape and patch them asap if something is broken.
I'm suggesting to open patch versions like:
qiskit-terra >= x.y
qiskit-aer >= x.y
...

So I believe @jaygambetta, @mtreinish, myself, and maybe @ajavadia already discussed this issue before we decided to pin the versions.
In essence, the reason behind the separate element repos is to give developers the flexibility to work on their projects without other, disjoint efforts getting in the way.  However we want to offer a coherent software suite.  If we let the versioning be as you propose, then we lose control over what a standard user has installed.  With version pinning we can say that we officially support this combo of packages, rather than we support any combination of various dependencies that satisfy >=.  Then when something is wrong, we can use the qiskit version as a guide to what versions of what to look in.    For example, take a look at Matlab (https://www.mathworks.com/help/matlab/ref/ver.html) where you see that their toolboxes (similar to our notion of element here) are tied to the core Matlab release (R2013b in the attached link).

yes @atilag I think the overhead of when you update and element increase and releasing an update of the the mega is less then the overhead of debuting the issues people have. You only need to connect your element to the mega not the other elements.
For this reason I would like to have it strongly correlated.


Incorrect metric Master.EmbeddedJournalSnapshotLastIndex and Master.JournalEntriesSinceCheckPoint

Alluxio Version:
Master branch commit id  601272157bf
Describe the bug
The metric Master.EmbeddedJournalSnapshotLastIndex and Master.JournalEntriesSinceCheckPoint are not correct.

Log from alluxio primary master, so i guess the current sequence_number is about 5448746
2021-08-10 03:03:21,757 WARN  SnapshotReplicationManager - Failed to request snapshot info from followers: alluxio.exception.status.UnavailableException: No recent snapshot found from followers

2021-08-13 14:10:20,105 TRACE RaftJournalWriter - Writing entry 5448746: update_inode {
  id: 19
  last_modification_time_ms:<PHONE_NUMBER>097
  last_access_time_ms:<PHONE_NUMBER>097
}

2021-08-13 14:10:20,105 TRACE RaftJournalWriter - Flushing entry journal_entries {
  sequence_number: 5448746
  update_inode {
    id: 19
    last_modification_time_ms:<PHONE_NUMBER>097
    last_access_time_ms:<PHONE_NUMBER>097
  }
} (Message:ba021908aac8cc029a02...(size=28))
2021-08-13 14:10:20,107 TRACE RaftJournalWriter - Writing entry 5448747: inode_directory_id_generator {
  container_id: 36004
  sequence_number: 152943
}

2021-08-13 14:10:20,108 TRACE RaftJournalWriter - Flushing entry journal_entries {
  sequence_number: 5448747
  inode_directory_id_generator {
    container_id: 36004
    sequence_number: 152943
  }
}
 (Message:ba020f08abc8cc025208...(size=18))

To Reproduce
Not sure
Expected behavior
Correct metric, no warn at this time
Urgency
High
Additional context
No

@LuQQiu seems like mSnapshotLastIndex is not getting updated even after a snapshot is taken? any ideas?

As we see the following logs, I suggest to use downloaded.getIndex() as lastSnapshotIndex.
2021-08-10 03:03:23,598 INFO  SnapshotReplicationManager - Completed storing snapshot at (t:4, i:3999999) to file /var/alluxio-share-dir/journal/raft/02511d47-d67c-49a3-9011-abb3109a44c1/sm/snapshot.4_3999999

@maobaolong Yeah, it make sense to compare downloaded vs current last snapshot index and use whichever larger

@LuQQiu The metrics lastAppliedTermIndex.getIndex() is not correct too.

@maobaolong can you give more information about what's the incorrect behavior of lastAppliedTermIndex.getIndex() after our fixes?
lastAppliedTermIndex.getIndex() is provided by Ratis and is not controlled by Alluxio.
Did you see a negative value?
I looked at the code and the logics seem to be correct