nbdev_create_config doesnt add black_formatting option

I am creating the config file via nbdev_create_config --black_formatting True but the option isn't added to the created settings.ini. I am a receiving a warning because I am using a private repo without having set the GITHUB_TOKEN but the same happens when I run the command in a completely empty repo.
On another note, it's not clear to me when the notebooks should be formatted? I hoped it would happen when saving the notebook but I couldnt trigger the black formatting after having added the setting manually?

The black_formatting option only applies to the exported Python files. The notebooks themselves don't get formatted.

Ah gotcha! About the first part though: Am I wrong in assuming that this command nbdev_create_config --black_formatting True should generate a config file with the option set to true?

Addressed by https://github.com/fastai/nbdev/commit/066a88ec1d8e6bf530692a4b754ac9bbf26ad2af


remove Nuclio location-based example

In Nuclio function example,

Real-Time Location-Based Recommendations  -> 404
The target notebook is already deleted.

/assign @sarahmaddox

Thanks @janeman98!
/lgtm
/approve

Thanks @sarahmaddox !


Page.move doesn't work without the pos argument

The tests fail too if it's omitted, e.g. if it's changed to events_index.move(about_us_page) here:
https://github.com/torchbox/wagtail/blob/master/wagtail/wagtailcore/tests/test_page_model.py#L350
with this error:
AssertionError: u'/home/events/' != '/home/about-us/events/
so the move itself fails silently.

The move command is behaving correctly here as the default value for "pos" is the next sibling. It just happens to be that the page is moving from that position so nothing appears to happen.


Fully conserve equality between generations by ordering #includes and class/struct pre-declerations

Tested on DRG, works. However, I think that the commandlet also needs to be updated since it is responsible for generating the engine module includes. But everything on the UE4SS side is done.
Obviously I am making sure that CoreMinimal.h and the generated.h includes are kept at top and bottom of include list respectively.

Tested properly with full build of new template instead of just up to the point of running the commandlet, all bugs fixed now, should be good to go.


Not working with PongDeterministic-v4

I tried running the code for PongDeterministic-v4 for 1.75 M steps but there was no improvement in score. It started with avg -21 but even after 1.75 M step the avg score was around -20.4. In the last testing phase too - the score was -21.0 throughout.
Any help with issue will be really appreciated.
Thanks

Which algorithm did you try for PongDeterministic-v4?

I used DQN for it. I used the default parameters of dqn_atari.py

I used DQN for it, with the default parameters of dqn_atari.py

Will have a look at it as soon as I get time.

@mirraaj I ran the same code on Atlantis-v0. It's not working there too. I saw that someone has reported similar issue for Enduro-v0 #83  too. It would be great if this could be looked into - it would be enormous help of many researchers out there. :)

@guptavaibhav18197 I guess the problem lies in exploration vs exploitation. When the reward is sparse, exploration becomes a necessity to get the best approximate of Q values. I am looking into the code base to check if there is a problem in the framework. Could you please let me know how long did you run the code and the total number of steps for which the code ran.

I guess you should start with Boxing-ram-v0 and Atlantis-ram-v0 to begin your experiments. These seem to be an easier task than the one you are trying. I do not see any problem with Boxing results. You can expect such decrease in performance once the DQN network starts overestimation. However, I am not convinced by the Atlantis results. Your graph shows that there is no improvement in the DQN network altogether. I am particularly not sure why this is happening. @matthiasplappert Could you please give your input :)

Hi there! I'm also experiencing similar results in Pong-v0, using dqn_atari.py . I have also tried modifying the script a little bit to follow more closely the experimental setup of the Nature paper, by using RMSprop(lr=.00025, rho=0.95, epsilon=0.01) instead of Adam. Also added BatchNormalization after each Conv2D+ReLU block, hoping it would help in stabilizing the training process.
Unfortunately after 5 million steps I can't see any progress.

Is this the expected result? The only feature mentioned in the Nature paper which I did not see implemented in dqn_atari.py (besides RMSProp) is some preprocess in which the value of each pixel in a frame is taken as the max between itself and the same pixel in the previous frame. This is reported to be useful to prevent Atari issues with flickering sprites. Could this be the reason? Am I missing some other critical point?

Yeah, I face the same issue. It seems that the network learns nothing. I just use the dqn_atari.py code in examples/ folder.

@wangjksjtu, Do you face the same issue when dealing with simpler environments like CartPole or MountainCar?

@wangjksjtu I will be looking into this issue in the coming weekend. Please let me know if you have any intuition what should be wrong. Also, please post the results for atari, if you have it.

@mirraaj, thanks a lot! If the code works for you, could you please share the settings or the code?
I trained the models with 1e7 steps. The results  (pong.pdf) are plotted by visualize_log.py script in examples/ folder. From episode rewards, we could know the model learned nothing.

@wangjksjtu Could you share your codes? I might want to have a look at it.

@mirraaj I just modify the env to PongNoFrameskip-v4 (dqn_atari.py in examples/ subfolder).

@mirraaj The code is as follows. Thanks a lot!
from __future__ import division
import argparse
import os

from PIL import Image
import numpy as np
import gym

import tensorflow as tf
from keras.models import Sequential
from keras.layers import Dense, Activation, Flatten, Convolution2D, Permute
from keras.optimizers import Adam
import keras.backend as K

from rl.agents.dqn import DQNAgent
from rl.policy import LinearAnnealedPolicy, BoltzmannQPolicy, EpsGreedyQPolicy
from rl.memory import SequentialMemory
from rl.core import Processor
from rl.callbacks import FileLogger, ModelIntervalCheckpoint

parser = argparse.ArgumentParser()
parser.add_argument('--log_dir', default='logs',
                    help='Log dir [default: logs]')
parser.add_argument('--mode', choices=['train', 'test'], default='train',
                    help='Mode (train/test) [default: train]')
parser.add_argument('--env-name', type=str, default='PongNoFrameskip-v4',
                    help='Atari Game [default: PongNoFrameskip-v4]')
parser.add_argument('--pre_trained', type=str, default=None,
                    help='The path of pre-trained weights [default: None]')
FLAGS = parser.parse_args()

MODE = FLAGS.mode
ENV_NAME = FLAGS.env_name
WEIGHTS = FLAGS.pre_trained
LOG_DIR = FLAGS.log_dir

INPUT_SHAPE = (84, 84)
WINDOW_LENGTH = 4

if not os.path.exists(LOG_DIR):
    os.makedirs(LOG_DIR)
os.system('cp dqn_atari.py %s' % (LOG_DIR)) # bkp of train procedure

class AtariProcessor(Processor):
    def process_observation(self, observation):
        assert observation.ndim == 3  # (height, width, channel)
        img = Image.fromarray(observation)
        img = img.resize(INPUT_SHAPE).convert('L')  # resize and convert to grayscale
        processed_observation = np.array(img)
        assert processed_observation.shape == INPUT_SHAPE
        return processed_observation.astype('uint8')  # saves storage in experience memory

    def process_state_batch(self, batch):
        # We could perform this processing step in `process_observation`. In this case, however,
        # we would need to store a `float32` array instead, which is 4x more memory intensive than
        # an `uint8` array. This matters if we store 1M observations.
        processed_batch = batch.astype('float32') / 255.
        return processed_batch

def train():
    # Get the environment and extract the number of actions.
    env = gym.make(ENV_NAME)
    np.random.seed(123)
    env.seed(123)
    nb_actions = env.action_space.n

    config = tf.ConfigProto()
    config.gpu_options.allow_growth = True
    sess = tf.Session(config=config)
    from keras import backend as K
    K.set_session(sess)

    # Next, we build our model. We use the same model that was described by Mnih et al. (2015).
    input_shape = (WINDOW_LENGTH,) + INPUT_SHAPE
    model = Sequential()
    if K.image_dim_ordering() == 'tf':
        # (width, height, channels)
        model.add(Permute((2, 3, 1), input_shape=input_shape))
    elif K.image_dim_ordering() == 'th':
        # (channels, width, height)
        model.add(Permute((1, 2, 3), input_shape=input_shape))
    else:
        raise RuntimeError('Unknown image_dim_ordering.')
    model.add(Convolution2D(32, 8, 8, subsample=(4, 4)))
    model.add(Activation('relu'))
    model.add(Convolution2D(64, 4, 4, subsample=(2, 2)))
    model.add(Activation('relu'))
    model.add(Convolution2D(64, 3, 3, subsample=(1, 1)))
    model.add(Activation('relu'))
    model.add(Flatten())
    model.add(Dense(512))
    model.add(Activation('relu'))
    model.add(Dense(nb_actions))
    model.add(Activation('linear'))
    model.summary()

    memory = SequentialMemory(limit=1000000, window_length=WINDOW_LENGTH)
    policy = LinearAnnealedPolicy(EpsGreedyQPolicy(), attr='eps', value_max=1., value_min=.1, value_test=.05,
                                  nb_steps=1000000)
    # policy = BoltzmannQPolicy(tau=1.)

    processor = AtariProcessor()
    dqn = DQNAgent(model=model, nb_actions=nb_actions, policy=policy, memory=memory,
		   processor=processor, nb_steps_warmup=50000, gamma=.99, target_model_update=10000,
		   train_interval=4, delta_clip=1.)
    dqn.compile(Adam(lr=.00025), metrics=['mae'])

    if MODE == 'train':
	weights_filename = os.path.join(LOG_DIR, 'dqn_{}_weights.h5f'.format(ENV_NAME))
	checkpoint_weights_filename = os.path.join(LOG_DIR, 'dqn_normal_' + ENV_NAME + '_weights_{step}.h5f')
	log_filename = os.path.join(LOG_DIR, 'dqn_{}_log.json'.format(ENV_NAME))
	callbacks = [ModelIntervalCheckpoint(checkpoint_weights_filename, interval=250000)]
	callbacks += [FileLogger(log_filename, interval=100)]
	dqn.fit(env, callbacks=callbacks, nb_steps=20000000, log_interval=10000, verbose=2)

	dqn.save_weights(weights_filename, overwrite=True)
	dqn.test(env, nb_episodes=10, visualize=False, verbose=2)

    elif MODE == 'test':
	weights_filename = os.path.join(LOG_DIR, 'dqn_{}_weights.h5f'.format(ENV_NAME))
	if WEIGHTS:
	    weights_filename = WEIGHTS
	    dqn.load_weights(weights_filename)
	    dqn.test(env, nb_episodes=10, visualize=False)

if __name__ == "__main__":
    train()

@mirraaj  Hi, do you figure out why it happens? Could you achieve convergence in you exps? Thank you.


floj wallet files are committed to GitHub repo

Should remove before repo gains public exposure or funds can be drained (if any)

This was not a problem, it was a small amount of FLO in there, feel free to use to test if anything shows up in there.

Okay, we can make use of it for testing.


Linear: add issueUnassignedFromYou notification type.

Adds a new notification type: issueUnassignedFromYou.
Before:

After:

Finally seeing all the work that I don't have to do 🤣 Great addition!


[BUG][Block 657887]  Native execution error: Native execution error: Native execution error: DIRECTION\n

When executing Block 657887 with starknet-replay, we get this error in certain transactions
Native execution error: Native execution error: Native execution error: Native execution error: DIRECTION\n

This error doesn't occur when we execute the block with only the cairo VM
transactions hash that triggers the error:

0x548c30d3ee15d5180c1cde487a7c4a66efd40dc8c3bad1afe9f98069523dcd4
0xd4e73c48b38d8394593c15b5c142d6d38ff16acf774453ae9b0f20dfc32c82

The same happens in the Block 659392
Transactions hashes:

0x1b2c498c06c6c3354c74d09dbc38de295cde904a2ef9bf10f8222ef9381be60
0x72e347547fb46f5ed1e23d84d72173da035fc4ad0c481f9564ca08beea73254
0x50614b4a762d3fecce94a58c2f63ed58665c0521e88e19f27894ed6217e069
0x6915b7ef6685ca9bcddb2b3b5d0007931fb91f9a85be1c636b5581257ffee62
0x78871ae0bb797ede5a80cd3659783ccf420d763505f1947af075622fb490ec0
0x357bf5d29d466fe2a1f9562a0373eb4c7c42411097eae267ac969be90b048f4
0x4b65ef109102f365244ff4613bd4f20c8ded773e530bb3ce993e9867385ca81
0x12880ce50c519bb6e959cbe71ea2ebdc5034a13874a08515210a8ffbb868b2e
0x19fbfbd511a8d87fb198860c251cb9e7baacda90badede1b52ba766346bdb8
0x58219ec9e52df2938e9b6ef18d626c35e1fdc92878a397c4aaec9ca2179d0f8
0x4194eca24d65b45b68ece95c5cee43fce0e17db20992f3e8cc50e570d239356
0x1baaf8b0de1a7d1dc5b70164f2406eef71e4f33985c183646dc002b08465736
0x4364a9864a2c1673c435ceee099de3cf3f559e4b2387fef3b5c86231fd486f2
0x56270b23b24d0ddffd93989252519e64b0cd43e6f5df88a666ccf0c871890ac
0x41ee056fc3399413d5cf5876e312bacc8e955ed89fdc9b62e346a9b26e50432

It also happens in other blocks, you can check in the execution logs inside the starknet-replay folder (in the server), files block-NUMBER.md

PR #778 fixes this bug. Some of those hashes still fail with other errors, but they also fail using the VM, therefore it may not be related to native specifically, but rather in the supported transaction versions or other compatibility issues.

Update
Same as #738

Fixed with #787 and #763


Node status error

Nodes are online but in embed it is showing offline

I'm sorry but this is not a node status bot


Correct mdsd image sha to distroless checksum

Which issue this PR addresses:
ARO-5049

Fixes
What this PR does / why we need it:
Correct the sha checksum for mdsd image to distroless version.

Test plan for issue:

Is there any documentation that needs to be updated for this PR?
No

How do you know this will function as expected in production?

/azp run ci,e2e


Regex can't match utf-8 characters

Hello,
I was trying to match some utf-8 characters but it doesn't seem to work.
/[[:alnum:]]/x.match "à" # => nil

It works in Ruby and seems compliant with PCRE, is there a bug with crystal or is there a subtlety I didn't notice here ?
Note: I also wrote an error which did not raise any alarm, a /[class]]/. Maybe it should be mandatory to escape the ']' inside or outside the class

This behavior related to
class Regex
  @[Flags]
  enum Options
    PCRE_UCP = 0x20000000

But this option requires PCRE build with UCP support.

Looks like somebody should replace PCRE with Onigmo.


⚠ Samsung has degraded performance

In 514dc70, Samsung (https://samsung.com) experienced degraded performance:

HTTP code: 200
Response time: 3197 ms

Resolved: Samsung performance has improved in c0ce923.


[Button] Icon after label removed?

Description
The code found here https://unpkg.com/browse/<EMAIL_ADDRESS>in the beta.5 release of the Button pattern persisted the ability to have an Icon after the label element in the button, but it seems to be removed in the most recent beta. Was this removal intentional, or will we see that returned in future releases? There's a little but of wonkiness in beta.6 where the selectors are duplicated in a way that doesn't seem required that makes me think this was maybe an oversight in the large amount of changes therein: https://unpkg.com/browse/<EMAIL_ADDRESS>Expected behavior
Styles for icon after label are available.
Environment

Spectrum CSS version<EMAIL_ADDRESS>-> beta.6
Browser(s) and OS(s): All

Yeah, it was removed, icon must come first. This is because t-shirt sizing changes lots of paddings as things get bigger, and to make it look right for the design, we had to make some assumptions about the order of the elements. The comments in the source file explain the logic. If there's a way to do this without having to force ordering of icon + label, I'm down!

Is there something I'm missing that prevents use of:
  .spectrum-Button-label + .spectrum-Icon {
    /* Have gap on on the left */
    padding-inline-start: var(--spectrum-button-primary-icon-gap);

    /*
      Have no padding on the right (it's built into the element)
      This will fail if --spectrum-button-primary-textonly-padding-right !== --spectrum-button-padding-right
    */
    padding-inline-end: 0;
  }

Similarly, how much longer until we can leverage gap? 😞 Seems like a while: https://caniuse.com/?search=gap unless we use display: grid over flex here...

Ah I see, you're right, there's no reason why we can't support right-side icons based on markup order if we do something like that.
You're saying you want this, but not ugly, right?

What the use case? I wonder if this would anger the design gods...

This is what we had before:

I can't say that I know where it came from, just that is as part of our library (and the Spectrum CSS output we built from). Beyond that I don't specifically have a use case that I know of...which is why it could be just a deprecation and I'd be OK with it.
The above CSS structure is roughly what we had been consuming before, if we wanted to continue to deliver it to end users.

So we've actually solved this like so: https://github.com/adobe/react-spectrum/blob/main/packages/%40adobe/spectrum-css-temp/components/button/index.css#L164

@Westbrook @snowystinger I'm confident we can add this functionality back (being able to order the 2 elements either way), but I wasn't sure it's something the design guidelines allows. I brought this up with the design team and they asked for some use cases. Do you have any you can post?

@Westbrook @snowystinger I'm confident we can add this functionality back (being able to order the 2 elements either way), but I wasn't sure it's something the design guidelines allows. I brought this up with the design team and they asked for some use cases. Do you have any you can post?

When we talked to design about it, they were clear that it should only have one visual ordering [Icon, Text], the other was invalid. I'm unaware of any use cases where it should be the other way around and no one has brought it up in the slack channel or issues.

When we talked to design about it, they were clear that it should only have one visual ordering [Icon, Text], the other was invalid. I'm unaware of any use cases where it should be the other way around and no one has brought it up in the slack channel or issues.

@snowystinger so when you said above you solved it and they are visually the same, does that mean that regardless of the dom order, it always shows the icon on left and label on right? or it allows the undocumented reversed order?

@snowystinger so when you said above you solved it and they are visually the same, does that mean that regardless of the dom order, it always shows the icon on left and label on right? or it allows the undocumented reversed order?

I'm also unsure/unaware of any usage. As noted above, I only noticed this change in that I was supporting the previously offered capabilities of Spectrum CSS.

I'm also unsure/unaware of any usage. As noted above, I only noticed this change in that I was supporting the previously offered capabilities of Spectrum CSS.

So we could force the order so it doesn't enable the reverse order, but it also doesn't exactly break people using it in an unsupported way. Would that help @Westbrook?

So we could force the order so it doesn't enable the reverse order, but it also doesn't exactly break people using it in an unsupported way. Would that help @Westbrook?

If the intent is to deprecate the usage of buttons with icons on the right, it would be no issue for us functionally. I'm only looking to confirm that's the intent and this wasn't a regression.

If the intent is to deprecate the usage of buttons with icons on the right, it would be no issue for us functionally. I'm only looking to confirm that's the intent and this wasn't a regression.

@Westbrook ok. I am closing it. My assumption, from speaking with the design team, is that it wasn't intentionally enabled (or shouldn't have been enabled) in the first place so it's not a regression it's just breaking an unsupported use case.

@Westbrook ok. I am closing it. My assumption, from speaking with the design team, is that it wasn't intentionally enabled (or shouldn't have been enabled) in the first place so it's not a regression it's just breaking an unsupported use case.

@GarthDB correct, no matter what order they are in the DOM, they will always render visually in the correct order

@GarthDB correct, no matter what order they are in the DOM, they will always render visually in the correct order


Discuss: Improving testing / integration / usage for 3rd party grammars

This is a continuation of #2328 (linking for reference).  Where-as that discussion was more about the file layout/structure of a 3rd party grammar and related conventions - this can be a discussion of how we continue to improve the tooling around working with 3rd party grammars - both as a user or developer.

If everyone is happy as things are then I may just put an auto-close clock on this one.

My original thought was to make it easier for grammar maintainers to "build" outside of our project - ie, not need their grammar checked out into our extras folder.  So perhaps you'd just set an ENV variable (or something to point to us) or  perhaps we'd provide some CLI tooling or something... but overall there seems to be little demand for this and I'd say out of every other thing we could possibly spend time on there are much more important things.
So I'm closing this issue for now.

Continued from https://github.com/highlightjs/highlight.js/issues/3008#issuecomment-789007258:

The disagreement is about whether the repository is the means of distribution, along with what I consider the normal distribution channels like GitHub Releases or packages on the npm registry.

@frangio I'm aware.  Some of our end users probably have no idea what a "npm registry" is... the desire is to make the binary as EASY to download (and use) as possible.  Someone could potentially:

Add dist (as we recommend) and let us build it (for guaranteed compatibility and consistency)
Use GitHub releases
Publish to NPM
Publish to a CDN as well

The bigger push here is that all grammars provide not ONLY a Node.js module, but also a CDN distributable [which includes the registration hook] as well and consistent instructions for using it easily (which MANY already do).  The CDN distributable should only require a single <script> tag to "install".  This serves the purposes of our many users doing simple website/CDN installs. (over 100 million per month)
These users have no build systems.  They perhaps don't know what npm is, nor would they care.
We of course encourage all 3rd party grammars to publish to npm so that anyone who uses a more complex build pipeline can integrate however they choose.

dist is our choice because that's what our built-in build system does (making it super easy for maintainers) and we want to see the install experience for all 3rd party grammars be consistent (a one liner script tag in the simplest case). It's also already a convention many grammars are already following.

I understand your points and if the suggestions work for the target audience that's great. I'm not criticizing at all. It was a misunderstanding on my part to think the proposed setup was a requirement where it's just a suggestion.

It was a misunderstanding on my part to think the proposed setup was a requirement where it's just a suggestion.

So far (since making providing of a CDN distributable itself a requirement) no one new has opted to do anything other than use our own build scripts and place it in dist.  I would suppose because that's what we've made super easy - and because maintaining a custom build system, GitHub Action, etc. is not everyone's cup of tea.
The first time anyone shows us something else in the concrete rather than abstract I'm sure we'll take a close look and see if it seems easy enough to use compared to everything else... though if more than a few confused users ever show up and ask "why do all the 3rd party grammars have dist folders but these 2" I'm not sure how sympathetic I'd be to the outliers.
So I'd say it's a very strong recommendation at this point. :-)


Iss 54

with this commit, there should be a working api for states (including data/migration): http://<IP_ADDRESS>:8000/jobs/state/


"import requests" breaks OpenSSL when used in mod_python

When creating a mod_python (Python 2.7, mod_python 3.3.1) page handler that just imports the requests module (version 2.12.3), it breaks pyOpenSSL (pyOpenSSL-16.2.0, cryptography-1.5.3, idna-2.1 in my case). For example creating certificates like this is not working anymore:
from OpenSSL import crypto
pkey = crypto.PKey()
pkey.generate_key(crypto.TYPE_RSA, 2048)

An exception like this is raised:  Error ([('rsa routines', 'RSA_BUILTIN_KEYGEN', 'BN lib')])
  File "key_mgmt.py", line 224, in _generate_key
    pkey.generate_key(crypto.TYPE_RSA, 2048)

  File "lib/python/pyOpenSSL-16.2.0-py2.7.egg/OpenSSL/crypto.py", line 258, in generate_key
    _openssl_assert(result == 1)

  File "lib/python/pyOpenSSL-16.2.0-py2.7.egg/OpenSSL/_util.py", line 61, in openssl_assert
    exception_from_error_queue(error)

  File "lib/python/pyOpenSSL-16.2.0-py2.7.egg/OpenSSL/_util.py", line 48, in exception_from_error_queue
    raise exception_type(errors)

When removing this from requests init.py, it works again:
try:
    from .packages.urllib3.contrib import pyopenssl as pyopenssl
    pyopenssl.inject_into_urllib3()
except ImportError:
    pass

I tried to remove only the pyopenssl.inject_into_urllib3() call, but that did not solve the issue. I had to remove the import which then fixed the issue. So it seems that it is somehow related to that module directly or some dependency.
The same code works when running as dedicated python script. Maybe it's related to the mod_python importer or something.
Any ideas?

This is definitely related to mod_python. The cryptography library which PyOpenSSL uses has had problems with mod_python in the past due to mod_python's unusual method of running Python. I suggest you open this issue on the pyca/cryptography repository: they should be best placed to fix this issue.


API research for long-running operations

We need to offer API that simplifies the task of executing long running operations against Azure. There is currently a draft spec proposal being discussed and reviewed. The ask of each language team is to provide at least a draft API proposal for how this will be fully implemented for the week starting May 26.
We should also review the existing polling code that exists in azure-core-management.
This issue therefore tracks this requirement in the first instance.

Quick review of the polling code that already exist and how it aligns with the proposal.
Querying the current operation state
All querying of the current operation state is reactive, sending status requests to the service.
Requesting an asynchronous notification when the operation has completed
Only Azure Core Management has the ability to receive a notification when the operation has completed.
Cancelling the operation if cancellation is supported by the service
There is no built in mechanism to cancel the polling operation.
Serializing the operation such that another process can pick up polling
Polling strategy implements Serializable which should allow for the polling to be portable.
Triggering a poll operation manually (automatic polling must be disabled)
Polling is automatically added to requests that allow for polling.

Though there are requirements missing in the current implementation I do believe it would be best to begin porting the polling logic from Azure Core Management into Azure Core, clean up the interface, add rich documentation, and make it generally available.

@bterlson  Question  on "5. serializing the operation such that another process can pick up polling".
Since We are adding callback functions  which poller will call when A.  polling complete and B. cancel the polling.
Does serializing requirement apply call back functions . In other words, should these callback functional needs to be serialized, deserialized back into poller object and resume polling ?

No, we should not need to serialize callbacks. The serialization is to enable easy passing of the serialized state to another VM (for example). I think it's something like a tuple of [StartOperationUrl, UpdateUrl, state]?

Thanks @bterlson for clarifying. Our initial design approach was very simplistic - have a Poller type that would be provided lambda expressions for doing a single poll, and to cancel the long running operation. The Poller, being instantiated by the client library, would know what to do using the API native to it. This means the Poller is relatively simplistic, calling the appropriate callbacks, and receiving them in a standard PollerResponse format (formatted by the client library within the lambda expression). This feels pretty decent, but it falls apart when including the serialization requirement in our consideration - because we aren't operating at the [startOperationUrl, UpdateUrl, state] level, and moving between the two levels might be troublesome. There are options we can explore, so this is just an FYI in case you've got anything you'd like to add.

If we stick to this design, I think we could still have a working approach, it just requires extra plumbing to enable serialization and deserialization. In the case of serialization, we can have the client library enable this inside the Poller by providing a callback that returns a SerializedPoller type, that contains appropriate fields such as operationName, clientLibraryName, lastKnownPollerState, as well as a deserialize() method that returns a Poller instance again.
To deserialize is a little more complex, and will lean on the Java service provide interface approach again. We will introduce an internal API called PollerDeserializer that client libraries can provide deserializers for. When a SerializedPoller requests deserialization, it calls into this API, and this API in turn checks if any client library will be able to handle the given SerializedPoller. Should one accept it, they can create a new Poller instance using the standard callback approach.
Whilst this sounds like a lot of work, the act of creating and deserializing Pollers inside a client library could quite easily be centralized into one place, lessening duplication of code.

Serialization has been punted from the V1 of LRO, from my understanding. Here is a gist outlining the current thinking for Java: https://gist.github.com/JonathanGiles/2a2cef537781fc7bd9ee1858097a03d0


fix/Docker_Image_Update_Cos

JIRA link (if applicable)
https://tools.hmcts.net/jira/browse/PRL-5721
Change description
Added a version tag to docker so it doesn't use the default latest
Does this PR introduce a breaking change? (check one with "x")
[ ] Yes
[x] No

https://tools.hmcts.net/jira/browse/VUL-6084 is not required anymore.


Use --force-window and --idle=once with mpv

Allows Syncplay to open mpv without having a media file ready:

This is cool.

Nice one. I'll look into that when I am next free. The important question is the version when the features wete added so Syncplay knows whether to use the functionality or not.

--force-window in 0.2.0, --idle=once hasn't been applied to 0.7.3, I think? but --idle has been there since mplayer.

Thanks for the intel. We try to maintain backwards compatibility within Syncplay so this kind of information is rather useful.

Just --idle seems to work too, so it's probably safe to use that.

Try https://github.com/Uriziel/syncplay/commit/a795acae0a2e8b19e50e0dfad34acbbeb17be585 and let me know if it works / if it causes any problems.

Seems to be working fine. I'll try a few episodes with friends tomorrow.

Quick fix: https://github.com/Uriziel/syncplay/commit/fd6d15790a93c3089b914c84e27a677951cecfa7

No bugs so far. I think it's safe to close this?

Yup, all seems good. Thanks for your contribution :)


Documentation updates

Documentation updates throughout the PoC.

Looks good - and we can clean up if not.

Looks good - and we can clean up if not.

There are some changes required actually... @alexgmetcalf

Oh well.  We can still change it :-)

Yeah, not a big deal. I have deployed the changes to dev so we can confirm everything looks as we expect it - when @alexgmetcalf confirms I'll push the changes forward to the remaining environments.


How to center and dynamically scale images to fit slide?

Hello,
I am trying to have images be centered in the slide and also scale to fit in the available space.
I've tried setting the div containing the img to display: flex and using justify-content: center.  For some reason I also need to have a div outside of that one, and then the centering works.  The problem is: when I scale the window's width to less than the image's width, I want the image to shrink and the slide to shrink with it.  Instead, the height of the slide never goes below the original image height.
Please see this CodeSandBox for a demonstration.  Try making the window less wide than the image to see what the problem is.
Help is appreciated.

not quite right... but should point you in the right direction:
https://codesandbox.io/s/7mmmly0rv1

I think this should be a manual CSS thing as you're doing in that codesandbox demo and nothing that we should implement internally. And if you tweak a little more with that CSS code, you'll achieve whatever you're trying to.

@fractalfrenzy you can use the className prop to define a custom class and then modify the css accordingly. This is how I achieved make the images smaller so that my custom arrows had more breathing room:
.productSlider {
  width: 100%;
  margin: 0 auto;

  .slick-slide {
    padding: 0px 75px;
  }

  .slick-next { right: 25px; }
  .slick-prev { left: 25px; }
}


Implement a new state store component for the Huawei DCS

Description
The Huawei DCS is implemented as a dapr state store component, and it wraps the existing Redis state store implementation.
The Distributed Caching Service (DCS) is a Huawei's cloud version of Redis. It is an online, distributed, in-memory cache service compatible with Redis on Huawei Cloud infrastructure, which is reliable, scalable, usable out of the box, and easy to manage, meeting your requirements for high read/write performance and fast data access.
See Huawei DCS User Guide for reference.
I will provide the documentation for this component usage later (similar to the setup-redis.md) and will raise a PR for that when the PR for this component is being approved.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #1596
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:

[x] Code compiles correctly
[x] Created/updated tests
[ ] Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
I will raise a PR for the documentation and will add an issue number here later.

Hi, I noticed that one of the checks failed in the CI, which says the "top level comment should end in a period". But, since this is the first time contribution to the Dapr, I am not sure what is the next step? should I fix this? I didn't get this kind of complaint when I run make lint in my local env.

Hi, I noticed that one of the checks failed in the CI, which says the "top level comment should end in a period". But, since this is the first time contribution to the Dapr, I am not sure what is the next step? should I fix this? I didn't get this kind of complaint when I run make lint in my local env.

Yes, you should do as the PR linter instructs. Thanks for your contribution!

Yes, you should do as the PR linter instructs. Thanks for your contribution!

I am not able to reproduce these linting errors when I run make lint locally (in windows env). Could you please give me some guidance on how to spot those linting errors as It would help me to ensure no linting errors before I push any code changes in the future? thanks.

Yes, you should do as the PR linter instructs. Thanks for your contribution!

I am not able to reproduce these linting errors when I run make lint locally (in windows env). Could you please give me some guidance on how to spot those linting errors as It would help me to ensure no linting errors before I push any code changes in the future? thanks.
Update: @yaron2 I don't even get any linting errors (specifically for this component) on linux env.

Just to give you some information about my linux environment, following is the ubuntu dev machine where I did a quick test and checked for linting errors:
Linux karthick-dev 4.15.0-38-generic #41-Ubuntu SMP Wed Oct 10 10:59:38 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

Yes, you should do as the PR linter instructs. Thanks for your contribution!

I am not able to reproduce these linting errors when I run make lint locally (in windows env). Could you please give me some guidance on how to spot those linting errors as It would help me to ensure no linting errors before I push any code changes in the future? thanks.
Update: @yaron2 I don't even get any linting errors (specifically for this component) on linux env.

Just to give you some information about my linux environment, following is the ubuntu dev machine where I did a quick test and checked for linting errors: Linux karthick-dev 4.15.0-38-generic #41-Ubuntu SMP Wed Oct 10 10:59:38 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

Are you using golint 1.31.0?

No, following are the go and golint versions which I am using in both platforms:

Env
Go
Golint

Linux (Ubuntu
go version go1.18 linux/amd64
golangci-lint has version 1.45.2 built from 8bdc4d3f on 2022-03-24T11:51:26Z

Windows
go version go1.17.2 windows/amd64
golangci-lint has version 1.43.0 built from 861262b7 on 2021-11-03T11:57:46Z

Golangci-lint is not backward compatible, so you need to use the same version, then will get the same result.

Golangci-lint is not backward compatible, so you need to use the same version, then will get the same result.

Pushed the changes with the fix for those linting errors (identified after I have downgraded the golint versrion to 1.31.0). Hope it should get through the failed check now.

@jasonlica lint passes! however there's a couple of tests failing.

@jasonlica lint passes! however there's a couple of tests failing.

Yes, those two tests cases (related to TestInit) are expected to fail when there is no redis instance setup in the environment.
I was taking the Redis state store as a reference for this component implementation, but I don't see any test cases written for the Init() in the redis_test.go. Also, it seems to be bit complicated to mock the actual Init() for unit testing. However, to keep the test coverage high,  I added those test cases which use the local redis instance for testing purpose.
Not sure whether it is really important to give a solution at this stage (ie., to mock Init() to keep the test cases passed), otherwise, I will probably comment out those two test cases for the time being.

Why is this PR and new component even necessary? If Huawei DCS is just Redis, can't we just update the Redis component if that is even necessary?
Also Huawei can just document which Huawai DCS settings correspond to which Redis settings. This really seems unnecessary to me because all this is does is mapping different metadata keys to existing redis options.
FYI @yaron2 @artursouza

This PR is identical to the way the Redis component behaves except that it maps:
"dcsHost" to "redisHost" and "dcsPassword" to "redisPassword"
That does not warrant a new component. This can just be explained in documentation...

Ping @jasonlica

Hey, @yaron2 @daixiang0 @daixiang,
Since all checks have passed, could you guys help approve and merge it.  Thx

@jasonlica please see the comment from maintainer @berndverst above.

Hey, @yaron2 @daixiang0 @DaiXiang,
Since all checks have passed, could you guys help approve and merge it. Thx

Please address my questions and concerns as mentioned before. https://github.com/dapr/components-contrib/pull/1628#issuecomment-1093393481
I'm one of the 3 maintainers of Component Contrib and I currently am against merging this PR as there is no technical justification for this component.

@berndverst @yaron2
Thanks for your comments.
Redis is a caching server which is not a Cloud Services.  It doesn't come with multi-tenant/instance etc. cloud related concept. It didn't have a way of specifying as cloud instance, and the cloud service specific attributes.  When a cloud console (e.g. Huawei DCS) is used, there are no way of differentiating between the Huawei DCS service instance and tenant.
Even though in current commit, we don't actually use  instanceName, instanceId, projectId properties, when it comes to real business scenarios, those Huawei DCS specific  properties  (i.e. instancdID, project ID, vpcID etc)will be required.
Thanks for your understanding.

@berndverst @yaron2
Thanks for your comments.
Redis is a caching server which is not a Cloud Services. It doesn't come with multi-tenant/instance etc. cloud related concept. It didn't have a way of specifying as cloud instance, and the cloud service specific attributes. When a cloud console (e.g. Huawei DCS) is used, there are no way of differentiating between the Huawei DCS service instance and tenant.
Even though in current commit, we don't actually use instanceName, instanceId, projectId properties, when it comes to real business scenarios, those Huawei DCS specific properties (i.e. instancdID, project ID, vpcID etc)will be required.
Thanks for your understanding.

Thanks for the explanation. Why not include the usage for these specific properties in the current commit? It would make perfect sense to accept this PR.

@berndverst @yaron2
Thanks for your comments.
Redis is a caching server which is not a Cloud Services. It doesn't come with multi-tenant/instance etc. cloud related concept. It didn't have a way of specifying as cloud instance, and the cloud service specific attributes. When a cloud console (e.g. Huawei DCS) is used, there are no way of differentiating between the Huawei DCS service instance and tenant.
Even though in current commit, we don't actually use instanceName, instanceId, projectId properties, when it comes to real business scenarios, those Huawei DCS specific properties (i.e. instancdID, project ID, vpcID etc)will be required.
Thanks for your understanding.

Thanks for the explanation. Why not include the usage for these specific properties in the current commit? It would make perfect sense to accept this PR.

Thanks @yaron2 .
As Dapr docs is stored in another independent rep, we are planning to push another PR (like other Dapr components) to dapr/docs repo to explain and illustrate the usage Huawei DCS (especially for those Huawei Cloud specific DCS properties).
Let me know if you still have any  other quesitons. Thanks for your help.
Dapr Docs Repo:  https://github.com/dapr/docs
Dapr Docs Website: https://docs.dapr.io/reference/components-reference/supported-state-stores/

@jasonlica I still don't understand.
The optional metadata fields are never used. The existing redis state store component you are calling unmarshals the metadata into a struct which has specific metadata fields it supports - your fields like VPCId are not supported there. So the Redis component isn't using this information or passing it on to Redis when creating a Redis client.
Similarly in your PR you never use the dcsMetadata struct at all after parsing the component metadata.
This means the DCS specific fields are never used anywhere and this component doesn't add any new functionality which does not exist in the existing Redis component.
If your goal is for your Redis Cache server to receive requests with the additional metadata field this PR does not currently do that.
@yaron2 please also take a look. Until the optional DCS fields are actually passed to Redis (they aren't in this PR) I do not think we should be merging this PR and adding this component.
@jasonlica once you update your PR to also send the optional DCS metadata fields I am happy to review again and approve this PR at this time. Currently this major aspect of your PR is completely missing.

Thx @berndverst . will update PR to reflect DCS specifc metadata usage. (currently on vacation)

@berndverst , @yaron2 , I understand your concerns where we have those Huawei DCS specific metadata fields which are kept unused other than those dcsHost and dcsPassword which are mapped to the respective Redis host and password fields. And, those metadata fields are just loaded into the DCS component even though they are not required for establishing a connection.
We have been evaluating number of use cases within Huawei which are very specific in nature and they require DCS specific attributes, such as, instanceId, projectId, etc. For example, query for the Huawei DCS instance once an application is deployed with a dapr sidecar and a DCS state store component into the Huawei Cloud's (K8s) env. And, in this particular scenario, we will be able to make use of the Dapr CRD endpoint to retrieve those DCS component metadata fields via the following URL exposed in the KubeApi server:
root@dapr-test-01313:~# kubectl get components
NAME         AGE
dcsstore     18h
statestore   20h
root@dapr-test-01313:~#

apis/dapr.io/v1alpha1/namespaces/default/components/dcsstore
Even though, it is completely possible to leverage the native Dapr Redis component for connecting to the Huawei DCS, we wouldn't be able to use it for the special use cases like above. Besides, we will have more flexibility if we have our own DCS component with its specific attributes as we continue to evaluate potential business use cases within Huawei. We will continue to evolve the DCS component in the future as we find more use cases in Huawei, and this component contribution could be a great beginning for that.
By the way, even though this is our first Huawei component contribution to the community, we will soon contributing a Dapr bindings component for the Huawei OBS.

@berndverst , @yaron2 , I understand your concerns where we have those Huawei DCS specific metadata fields which are kept unused other than those dcsHost and dcsPassword which are mapped to the respective Redis host and password fields. And, those metadata fields are just loaded into the DCS component even though they are not required for establishing a connection.
We have been evaluating number of use cases within Huawei which are very specific in nature and they require DCS specific attributes, such as, instanceId, projectId, etc. For example, query for the Huawei DCS instance once an application is deployed with a dapr sidecar and a DCS state store component into the Huawei Cloud's (K8s) env. And, in this particular scenario, we will be able to make use of the Dapr CRD endpoint to retrieve those DCS component metadata fields via the following URL exposed in the KubeApi server:
root@dapr-test-01313:~# kubectl get components
NAME         AGE
dcsstore     18h
statestore   20h
root@dapr-test-01313:~#

apis/dapr.io/v1alpha1/namespaces/default/components/dcsstore
Even though, it is completely possible to leverage the native Dapr Redis component for connecting to the Huawei DCS, we wouldn't be able to use it for the special use cases like above. Besides, we will have more flexibility if we have our own DCS component with its specific attributes as we continue to evaluate potential business use cases within Huawei. We will continue to evolve the DCS component in the future as we find more use cases in Huawei, and this component contribution could be a great beginning for that.
By the way, even though this is our first Huawei component contribution to the community, we will soon contributing a Dapr bindings component for the Huawei OBS.

Thanks for the explanation @jasonlica, and we appreciate the effort and contributions Huawei are making to Dapr. It's very problematic for us as maintainers who need to ensure constant testing, maintenance, compliance and certification for components to accept non-functional changes to a component.
For the use case you specified, please know that you can use your own custom metadata on any Dapr component today without any issues..
If you put a dcsHost metadata property on the existing Redis component, you would still be able to get it's value with mentioned kubectl command.

Thanks @yaron2 for the information. I will close this PR and the respective proposal #1596.


Different color for test files in quick pick, explorer, peek reference view, scm view, etc.

It would really help to have a different color for test files so it's easier to distinguish like it's in WebStorm. You could see it in quick pick, explorer, peek reference view, scm and so on. The question is, how to determine the test file since every language uses their own names and extensions. Probably use a most common patterns for each language? Or have a separate config to define these paths ourself like we have for fileNesting feature? This probably is open to discussion which way is a better.
VS Code

WebStorm
Find usages:

Git:

Explorer:

Quick pick:

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.
Happy Coding!

This feature request has not yet received the 20 community upvotes it takes to make to our backlog. 10 days to go. To learn more about how we handle feature requests, please see our documentation.
Happy Coding!

:slightly_frowning_face: In the last 60 days, this feature request has received less than 20 community upvotes and we closed it. Still a big Thank You to you for taking the time to create this issue! To learn more about how we handle feature requests, please see our documentation.
Happy Coding!


Remove the IControllerFactory.ReleaseController

In ASP.NET 5, the IControllerFactory isn't responsible anymore for creation controllers. This responsibility is moved on to the IControllerActivator abstraction (which is passed in the controller Type). Since it now is the IControllerActivator that is responsible of creating types, it should also be responsible of making sure it is 'released' or disposed properly. That basically means that the IControllerActivator should have a ReleaseController method as well and the IControllerFactory implementation should delegate calls to IControllerFactory.ReleaseController to IControllerActivator.ReleaseController.
This however would be pretty awkward, and now we have to equal points to override ReleaseController. So a much better design would be to remove ReleaseController altogether. A ReleaseController method is redundant, even with DI frameworks (i.e. Castle Windsor) where object must be released explicitly.
Since IControllerActivator is responsible for creating controllers, it should as well be made responsible of disposing/releasing controllers. Most frameworks don't need this, but the Castle implementation can ensure deterministic release by wrapping the controller into special 'release wrapper' object that itself is registered for disposal at the end of the request. This object can call the container's Release method while passing on the controller. This allows the container to decide what has to be done with the controller. In case the controller is scoped and implements IDisposable, it can be disposed.
Another point is that with ASP.NET 5, controllers don't have to inherit from Controller of IController anymore, which is awesome btw. This allows keeping controllers thin and even stateless, which can improve performance. However, in case a user decides to register (some of) his controllers as singleton, the DefaultControllerFactory will currently dispose his controllers at the end of the request, while that controller is intended to be reused. In other words, the underlying DI container isn't informed about this and is completely left out of the equation, which is bad, since it is the DI container that should decide whether or not an object is reused or disposed.
So long story short: The IControllerFactory.ReleaseController method should be removed.

This is covered in my article on DI-friendly frameworks. In short, lifetime management is a single responsibility; this includes not only creation of objects, but also decommissioning.
A framework API is easiest to understand if the object that creates other objects is also responsible for decommissioning them. So, if IControllerActivator creates Controllers, it should also release them when it no longer has need for them.
The ASP.NET MVC 1 & 2 IControllerFactory interface was perfect:
public interface IControllerFactory
{
    IController CreateController(
        RequestContext requestContext,
        string controllerName);

    void ReleaseController(IController controller);
}

It doesn't matter what you call such an interface, or exactly which parameters are passed; what matters is that this interface clearly describes the lifetime model used by the framework.
As a client developer, when I'm given such an interface, I don't even have to read the documentation; it's immediately clear to me how object lifetime is being managed.

Most frameworks don't need this

I'm not sure that I agree with this. If you have a nested object graph where one of the leaf nodes are disposable, there are only two ways it can go:

either you dispose of that object when its lifetime is up
or you let it stick around until its finalizer is run by the garbage collector

The second option is a poor use of resources, and will impact the capacity of the system employing it. Thus, the only safe, efficient choice is to dispose of objects when their lifetimes are over.
For any object with either Transient or Request-bound lifetime, the framework has to provide a deterministic callback where objects can be disposed of.
It's not that Castle Windsor is being difficult in this regard. On the contrary, it's one of only a few DI Containers that actually take this problem seriously. As a contrast, StructureMap has no decommissioning support, so if object graphs contain disposable nodes, the application will have resource leaks.
To be clear: making Controllers implement IDisposable doesn't address the decommissioning problem, because you may have dependencies of dependencies, and a Controller will not be able to tell if some concrete implementation in the dependency graph needs to be disposed of.
Notice that I consciously use the term decommissioning where appropriate. The reason is that even if an object implements IDisposable, it may not mean that is should be disposed of. If, for example, that object has Singleton lifetime, it should only be disposed of when the entire application shuts down.
This is a more complex topic than most people seem to realise, but it doesn't have to be that difficult. I should hope that chapter 8 of my book does a good job of explaining the issues involved.

Most frameworks don't need this

I'm not sure that I agree with this.

I'm sorry, I didn't make myself very clear. The reason most DI frameworks don't require explicit release, is because they will release objects for you when the scope (in ASP.NET, a web request) ends. So either the infrastructure will dispose this 'scope' because an Request_Ends event got raised, or the user will call Dispose once on the scope (which is the case with Autofac's container.BeginLifetimeScope()) (and this would hopefully be done by the user inside the composition root).
So releasing objects IS required and should not be ignored, but my argument is that a ReleaseController method is never required, because the DI container's integration package for ASP.NET ensures releasing at the end of the request for us. For the Castle integration package that means a bit more (but not much) work to make sure controllers are released, compared to the other frameworks. But as I explained, implementing this in Castle is actually trivial. So IMO, all containers actually take this problem seriously, but most manage this for you automatically.

Well, I was never a fan of lifetime scopes, but they're functionally equivalent to Resolve/Release. It's the same model, you're only moving the methods around a bit.
"ReleaseController is never required" is true only under the assumption that there's a scope that ends when the request ends. The framework doesn't call ReleaseController, but it calls Dispose on the scope. Isn't the difference purely semantic?

That said, I may have been confused by the wording. In order to make this discussion clearer to readers, can we please agree on the following definitions?

A Library is a reusable set of types or functions you can use from a wide variety of applications. The application code initiates communication with the library and invokes it.
A Framework consists of one or more libraries, but the difference is that Inversion of Control applies. The application registers with the framework (often by implementing one or more interfaces), and the framework calls into the application, which may call back into the framework.

In this discussion, ASP.NET is a framework. All DI Containers I'm aware of are libraries.

In this discussion, ASP.NET is a framework. All DI Containers I'm aware of are libraries.

I updated my initial question to make this more clear.

"ReleaseController is never required" is true only under the assumption that there's a scope that ends when the request ends.

In the case of ASP.NET, there is always a scope when the request ends (since the request is the scope, and we can always be notified when this request is about to end). So for ASP.NET the ReleaseController method actually is redundant. Not only is it redundant, we now have an extra method on an interface that must be implemented when a custom implementation is made, and the current behavior of the ASP.NET framework is plain wrong, which makes it harder to create a custom implementation (it requires overriding both IControllerFactory and IControllerActivator).

This does seem wrong - the contract for creating and destroying controllers should be the same.

I believe there is another issue filed to give both the IControllerFactory and IControllerActivator a create and release method - so I'm going to be closing this one since it's dealing with a recommendation that should continue over there if needed.
To introduce IoC type activation you only need to override the IControllerActivator. The IControllerFactory calls through to it, and also implements the rest of the work to contextualize the instance after it's instantiated. Things like initializing controller and action context properties, etc.
The reason the activator needs release is that the controllers are not resolved as IoC services by default but the user may choose to implement IDisposable which must be called after the request is ended. The default IControllerActivator will call dispose explicitly on Release, but an IoC-based activator only needs to resolve the controller in create and does not need to do anything in release. As you've observed - they can be scoped services and the container will dispose them at the appropriate time.

I believe there is another issue filed to give both the IControllerFactory and IControllerActivator a create and release method

Link please?

@lodejard what if we just registered controllers as services as part of the scan? It might makes things a little awkward since it happens during container build up time.

I believe there is another issue filed

Which one?

The reason the activator needs release is that the controllers are not resolved as IoC services by default but the user may choose to implement IDisposable

The fundamental reason a decommissioning hook is required isn't particularly to address Controllers that implement IDisposable, but to address the common scenario where a Controller has been injected with an object graph, and deep in that object graph exists an object that should be disposed of.
A common architecture is to have a Controller talk to an injected Domain Service, that again talks to an injected Repository:
FooController has an injected IFooService object. IFooService is an interface, but at run-time, the concrete implementation FooService is used.
FooService depends on IFooRepository (another interface), which is injected into it. At run-time, the concrete class EntityFrameworkFooRepository is used, and this object in turn contains an injected DbContext-derived object.
To summarise, here's the dependency graph:
FooController -> FooService -> EntityFrameworkFooRepository -> FooContext
FooController only knows about IFooService, which is an interface. An interface shouldn't derive from IDisposable as Nicholas Blumhardt already established in 2010:

an interface [...] generally shouldn't be disposable. There's no way for the one defining an interface to foresee all possible implementations of it - you can always come up with a disposable implementation of practically any interface.

This means that FooController only knows that it has a reference to an interface. The rest of the dependency graph is hidden to it, and it doesn't know if any nodes in this graph are disposable.
The same argument applies to FooService. It can't see beyond its IFooRepository dependency.
Still, when FooController goes out of scope, it's important to dispose of FooContext, because otherwise the application will have resource leaks.
The only object that knows about the entire dependency graph is the factory that created it. This is the reason that any API responsible for creating an object graph must also have a decommissioning hook (e.g. a Release method).
All of this is made even more complicated by the dimension of lifetime management. Some object graphs may contain nodes that are disposable, but which are also thread-safe. When you have such objects, it makes sense to give them Singleton lifetime. The result of that is that even though they implement IDisposable, they shouldn't be disposed of after each request.
So you may have dependency graphs where some nodes aren't disposable at all, some nodes are disposable and should be disposed of, and some nodes are disposable, but shouldn't be disposed of yet.
All of this should be of no concern to the rest of the framework that uses Controllers. All it needs to do is to call Create when it needs a Controller, and Release when it's done with it. The Composer on which the framework calls Create and Release takes care of the rest.
(For small object graphs it's actually easy and beneficial to juggle this lifetime management with 'normal' code, but you can always delegate such work to a dedicated DI Container like Simple Injector or Castle Windsor.)

the controllers are not resolved as IoC services by default

This seems to be a design oversight IMO, because controllers are application components and the container (either the built-in one or a custom one) should be in control of the creation of a controller. This has to be, because a user might want to:

control the lifetime of a controller (make it singleton for instance)
let a controller have multiple constructors (as some DI containers support). Currently even the built-in container supports multiple constructors, but MVC again forces a single constructor; this is inconsistent and confusing.
use property or method injection
let the controllers be part of the application's object graph so analysis and diagnostics can be performed by the container about the correctness of the configuration.
inject a dependency based on the controller it is injected into.

This all isn't possible with the out-of-the-box behavior of MVC, which makes it really confusing for the user. While .NET Core contains its own built-in container, MVC again defines its own 'container' with different behavior.
There is no doubt in my mind that this behavior should be completely removed. MVC should out-of-the-box simply delegate the creation of controllers to the built-in container. I don't see any good argument why MVC should be in control over the creation.

the user may choose to implement IDisposable which must be called after the request is ended

This is incorrect; Dispose needs to be called when the application is done using the controller, not after each request. If the controller is a singleton, calling Dispose after a request will clearly cause the application to break.


Create ru.json

Add Russian Translation
Vote button should be appears with label, but now hardcoded.

@RonLek @murtaza98 please consider to merge my PR and add support of Russian language in code.

Thanks for the awesome translations @ankar84 . Could you please raise this PR within the new Poll Plus App. It's the an update over this Poll App.
There's also a feature request issue by one of the users for this.

Could you please raise this PR within the new Poll Plus App?

Sure!
https://github.com/Poll-Plus/rocket.chat.app-poll/pull/4


IE11 does not display emojicon correctly, Chrome working fine.

We are using ng-Embed in our application. It works fine in Chrome, but IE 11 it just shows black-n-white emojicons.

show the code and the text that you are trying to convert.

I have added ng-embed library. and all other required stuff also.
code for binding is like this

I am selecting emojicon from other tool, the output is like this
😂
When I save above, it send correct emojis to Facebook.
My concern is that all works perfectly in Chrome but not in IE.
your demo i.e. http://riteshkr.com/ng-embed/ also not render emojicons properly.
May be fontawesome problem?

@ritz078 any update on this?

I will need help on this. I don't have a windows. So if you find a bug can you please submit a PR ?


Redundant booleans

Here's my first pass at collapsing what I'm calling "redundant booleans," ie:
if expr, do: true, else: false

I don't quite like that name and am still thinking on it but I needed something!  It only takes case of do: true, else: false because I realized it could be taken further but I decided that isn't worth it.  do: false, else: true could protentially be re-written with the if expression negated.  You have to chose between ! and not (possibly making it configurable) and it seems like one of those things that should just be a credo warning.
I only dealt with leading comments on the if line though I can be more thorough.  Again, it just didn't seem worth it since people usually write these "redundant booleans" pretty absent-mindedly and can't imagine they'd be full of comments in the do/else bodies.  I'm also not exactly sure how to keep comments on the same line.  "Leading comments" are comments both above and after a line.  I'll study Sourceror a little more but I just wanted to get this up for review.
I'm certainly open to different ideas on the name!  I was even thinking something as dumb as NoDoTrueElseFalse 😅 😅 😅
Thanks!

Thanks a lot. I will take a look at the weekend.

Already not looked at the code, but my suggestion for the name would be UnnecessaryIfUnless. As the name suggests, the implementation should also include unless.
To rewrite if expr, do: false, else: true to not expr would be nice, but we can leave this for later (or never).

All feedback has been addressed including rewriting the corresponding unlesses.
I'm happy to squash a succinct commit if you're happy with it!

Pull Request Test Coverage Report for Build 298454e56b33de8ccb84c5e6d60c1c5841f3358b-PR-89
Details

20 of 21 (95.24%)  changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.08%) to 91.5%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

lib/recode/task/unnecessary_if_unless.ex
20
21
95.24%

Totals

Change from base Build a97ec42c4b51b478e237027cea09efa4e41f3539:
0.08%

Covered Lines:
958

Relevant Lines:
1047

💛  - Coveralls

Do you want me to squash this or does your CI handle that?

I will handle this with GitHub.


Hotkey overlap issue

Describe the bug
V and P hotkeys are reserved for the Move and Polygon region tools respectively. At the same time, these hotkeys may be automatically generated for Labels  https://github.com/HumanSignal/label-studio-frontend/blob/master/src/core/Hotkey.ts#L352.
In such cases, the tool hotkeys have priority over Label hotkeys.
Screenshots

Hi @MorganFujimaka - nice catch here, this is a bug for sure. We'll file a ticket and get it fixed!


add role-configs command to list db roles with custom settings

What kind of change does this PR introduce?
Feature
What is the current behavior?
Please link any relevant issues here.
What is the new behavior?
New role-configs command under inspect to show all users that have a configuration that is different from the standard
Additional context
Useful for debugging and auditing

Pull Request Test Coverage Report for Build<PHONE_NUMBER>1
Details

15 of 26 (57.69%)  changed or added relevant lines in 2 files are covered.
5 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.03%) to 59.796%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

cmd/inspect.go
1
4
25.0%

internal/inspect/role_configs/role_configs.go
14
22
63.64%

Files with Coverage Reduction
New Missed Lines
%

internal/gen/keys/keys.go
5
12.9%

Totals

Change from base Build<PHONE_NUMBER>7:
-0.03%

Covered Lines:
6376

Relevant Lines:
10663

💛  - Coveralls


Documentation

Summary
Documentation on functions similar to other Discord Libraries.
Intended Outcome
Docs in production which can be used to find functions or how certain functions work.
How will it work?
Software such as WikiJS or Docasasaurus, etc.
Code of Conduct

[X] I agree to follow this project's Code of Conduct

@Techout592 I'm starting to work on this issue. I'll have a site in beta with documentation.


Auth provider - SAML auth summary page styling issue

Rancher version: v2.5-head (02/01/2021) 610920447
When enabling a SAML authentication a summary page is seen with the configuration information.
But it's a bit hard to read, we should add some styling to each field (bold each one ?)

And it looks User Name Field is missing a space after the colon (see screenshot).

Rancher version: v2.5-head (02/22/2021) a85f4d1
The summary is much more clearer now, and the text is arranged in responsive way.


[auto-merge] branch-24.10 to branch-24.12 [skip ci] [bot]

auto-merge triggered by github actions on branch-24.10 to create a PR keeping branch-24.12 up-to-date. If this PR is unable to be merged due to conflicts, it will remain open until manually fix.

FAILURE - Unable to auto-merge. Manual operation is required.
{'message': 'Pull Request is not mergeable', 'documentation_url': 'https://docs.github.com/rest/pulls/pulls#merge-a-pull-request', 'status': '405'}

Please use the following steps to fix the merge conflicts manually:
# Assume upstream is NVIDIA/spark-rapids remote
git fetch upstream branch-24.10 branch-24.12
git checkout -b fix-auto-merge-conflict-11546 upstream/branch-24.12
git merge upstream/branch-24.10
# Fix any merge conflicts caused by this merge
git commit -am "Merge branch-24.10 into branch-24.12"
git push <personal fork> fix-auto-merge-conflict-11546
# Open a PR targets NVIDIA/spark-rapids branch-24.12

IMPORTANT: Before merging this PR, be sure to change the merging strategy to Create a merge commit (repo admin only).
Once this PR is merged, the auto-merge PR should automatically be closed since it contains the same commit hashes

@res-life please help resolve the merge conflict when available, thanks


How is this project coming along?

So, I am a relatively new Crystal coder, but been a Ruby one for much longer. I see this is the most active tree-sitter of crystal and I wanted to ask what the status was? I wanted to fork and see if I can help, as I am looking into wrapping a lot of C code in a game engine project I am working on crystal. Is the grammar implementation complete, have you considered this?
https://crystal-lang.org/reference/1.10/syntax_and_semantics/type_grammar.html
I don't quite know much to be fair, beyond a few CLI and programming syntax stuff, I wouldn't say I 'know' Crystal like Ruby in regards to runtime/comptime but I manage.
Let me know any questions of interest you may have, currently finishing up 2 crystal books and might also do a solid web app to get things moving myself soon but if there anything big or small I can help with the project, you can ask.

@UltimaTerra Thanks for asking! I know the project status isn't very transparent, since I haven't created a proper readme or anything. I'd estimate the tree-sitter grammar is roughly 75% done. I'm still continuing to work on it in my free time. The type grammar you linked is fully implemented.
Some of the remaining bits of work are:

Everything related to C bindings (lib blocks, fun definitions, C-type structs, etc.)
Some miscellaneous keywords like super and uninitialized
Macros

Macros in particular are going to be tricky to handle correctly, since they can take the place of any other AST node. I still haven't figured out the best way to do this.
If you're interested in contributing, I suggest reading the TS docs and then searching the code for TODO items. You could also find a complex Crystal file and try parsing it to see what breaks.

I just forked it, I will be doing just that. Is there any generlizations I can use to get started? I have the treesitter docs and the code. I want to assume just parsing for files see if there is any clash with the binding itself. I guess write something complex in crystal, then parse it with treesitter see if something breaks? Is the binding for c-type structs related to structs in crystal? (I would rather just do 1 to 1), same with function declarations.
Macros are interesting, are you saying it is tricky because of the low level nature of C or the more higher level nature of Crystal with AST replacements? I have experience in Nim macros, I haven't actually tried many with Crystal so I suppose that will be fun to look at.
Sorry if this is alot. Just assume I want to help, but I am still somewhat noobish. I hope these are the right questions/observations.


Coverage instructions in README.md

Just what I was able to piece together. Let me know if I'm missing anything or if something's incorrect.

Coverage remained the same at 99.133% when pulling fd4a77a63b3125dbd428749122f4ec451d325f75 on ajfriend:cov into cd34251c87cae84998b06053fe9230ab1c939339 on uber:master.


[RUNTIME][CONTRIB] CoreML Runtime

RFC: https://discuss.tvm.ai/t/rfc-coreml-runtime/6309
This PR introduces a runtime to load and execute CoreML models from TVM.  To run CoreML inference via RPC, please setup iOS TVM RPC beforehand.
This is useful for a baseline benchmark measurement.  In addition, we could extract a subgraph and offload it to CoreML with this runtime - it would be important for optimization on iOS since using CoreML is the only way to leverage Apple's Neural Engine.
@tqchen @ZihengJiang @FrozenGene  Could you help to review this?

Thanks @kazum ! given that it is a new runtime, it would be great if you can send an RFC :)

I've created RFC https://discuss.tvm.ai/t/rfc-coreml-runtime/6309.

cc @FrozenGene @tmoreau89 @ZihengJiang

Is is ready to review?

@FrozenGene  Yes, please help to review :)

Thanks @kazum Wish we could have one end2end tutorial next, especially how to leverage iPhone NPU Engine. I think this will attract many people, especially for Mobile developers.


Analytics/Metrics/Monitoring

 Issue by Twinski
Thursday Jan 12, 2017 at 10:48 GMT
Originally opened as https://github.com/graphcool/prisma/issues/63

Would be cool to monitor the speed/performance of a specific GraphQL query. To see how many effective queries were executed under the hood.
( for example: https://youtu.be/c35bj1AT3X8?t=41m39s )
This makes sure the developer knows what happens under the hood instead of just relying on 'magic'. :)

Maybe the amount of queries interests me & also execution time in total (and even better, per individual query under the hood)

Any update on this issue? My users are starting to experience large load times and I'm trying to understand the issue.

apollo has performance monitoring, any chance that can be integrated?


arabic: Add symbol layer from AOSP layout

Includes diacritics and a few essential symbols. This should address most of the criticism, but don’t make it the default layout.

Great. I'll deploy later tomorrow.


getInputProps might be better solely as getValidationHandlers

So this is just a little thought I wanted to share.
In my opinion the naming getInputProps is a little confusing, as it's main role is to simply pass through the event handlers for validation.
Given the below code:
<ValidatedForm validator={loginValidator} method="post" >
	<FormInput
		label="Email address"
		name="email"
		type="email"
		autoComplete="email"
		required
	/>
	<FormInput
		label="Password"
		name="password"
		type="password"
		autoComplete="current-password"
		required
	/>
      <button type="submit">
	      Sign in
      </button>
</ValidatedForm>

As it stands without knowing, I would expect to see all the additional props; type,  autoComplete  etc, come through.
In reality it would return the 2 event handlers and defaultValue.
With no ability to actually add any more props there's also no reason defaultValue should come through by default, as you have access to it seperately out of useField and can choose what to do with it there.
Let me know whether you agree :D

Hi! Thanks for raising your concerns. 😄
I must admit I'm a little confused by your example since you're not showing a usage of getInputProps. I think it would be helpful for me if you could show me what your code for FormInput looks like.
My hope behind the name getInputProps is to communicate that it's using the prop getter pattern. The goal of this is to make it so you don't really have to know or care which props getInputProps is returning.
For example, If you do this
<input
  {...getInputProps({
    type: 'email',
    autoComplete: 'email',
    required: true,
    onBlur: () => doSomething(),
  })
/>

Then it would output all of the props you passed in combined with the props getInputProps is providing.
Which is why I'm also a little confused by this point you made:

As it stands without knowing, I would expect to see all the additional props; type, autoComplete etc, come through.

because that's exactly the goal / expectation of this pattern.

Oh Interesting! Unless i'm missing it, there's no explanation of that functionality / pattern / expectation in your docs? :(
I guess it's simple enough to retain type safety via this:
<input
  {...getInputProps<HTMLProps<HTMLInputElement>>({
    type: 'email',
    autoComplete: 'email',
    required: true,
    onBlur: () => doSomething(),
  })
/>

It's more a question of, why send every prop through to this function that only touches 2 event handlers.
I'm not sure if this method would work, but i'd rather the library do less work, and I get to keep native HTML like this:
const { defaultValue, validationHandler } = useField(name);

return (
  <input
    name={name}
    id={id}
    type="email"
    autoComplete="email"
    required
    onBlur={validationHandler}
    onChange={validationHandler}
  />
);

That way the user can just decide where and how to trigger validation, extend it however they please, and there's zero markup change.
Especially useful for existing projects that want to bring in the validation, and creates a lot more flexibility

Just realising the above suggestion is possible with validate already!
I wonder whether the Docs should show the setup without getInputProps first, so that the users understand what's going on before abstracting it away into a helper function?

Oh Interesting! Unless i'm missing it, there's no explanation of that functionality / pattern / expectation in your docs? :(

This could probably be made clearer. I do mention the term prop-getter in the API reference but don't really give a best practice on how to use it.

I guess it's simple enough to retain type safety via this:

Type safety should already "just work" if you're spreading the props into the input (is it broken?). I'll be publishing some changes this week that should clean up the type definition a little more, too.

I'm not sure if this method would work, but i'd rather the library do less work, and I get to keep native HTML like this:

I wonder whether the Docs should show the setup without getInputProps first, so that the users understand what's going on before abstracting it away into a helper function?

This was how things worked before I added getInputProps. Once I added support for tracking touched states it seemed like a lot of boilerplate to set up right off the bat. It's tricky to find the balance between "get up and running fast" and "here's everything that's going on".

Yeah okay so everything is fine in the package, and it's just clarity in the docs that's needed 😛

Type safety should already "just work" if you're spreading the props into the input (is it broken?). I'll be publishing some changes this week that should clean up the type definition a little more, too.

You are right, not sure why it wasn't popping up before 👀

This was how things worked before I added getInputProps. Once I added support for tracking touched states it seemed like a lot of boilerplate to set up right off the bat. It's tricky to find the balance between "get up and running fast" and "here's everything that's going on".

Yeah I think it's just offering the explanation and options to the dev so they know what's happening and can make an informed choice which path to take 👍
Anyways thanks for the quick replies! I hope this was informative / helpful in some capacity 😄


refactor command tracking to be per shell

atm, commands are tracked per terminal instance

Tracked in https://github.com/microsoft/vscode/issues/141006