securityContext set on worker pod making it difficult to use on OpenShift

Hi There!
Recently you merged a PR I opened to make the securityContext: configurable for the controller which greatly simplifies deploying the operator to OpenShift clusters, that worked well but issue I have now is the same securityContext is set by the controller on the workflow pod that it spawns to run the tf workflow
Is there anyway you can make that configurable or maybe inherit the securityContext from the controller?
The reason this is an issue on OpenShift is that OpenSHift has this concept of SecurityContextCOnstraints and defining an arbitrary runAsUser is a privileged action, it is possible to get around this by creating a dedicated SCC for the service account the terraform-operator users for the workflow pod, granting it access to use "Any Run as user" but better practice is to not set that value which allows OpenShift to set its own randomly allocated UID from the range assigned to the particular node
2022-11-15T14:42:30.080Z	DEBUG	controller-runtime.manager.events	Warning	{"object": {"kind":"Terraform","namespace":"stakater-terraform","name":"simple-template-example","uid":"6561f384-b5ec-448e-a01a-10aeda23b2e9","apiVersion":"tf.isaaguilar.com/v1alpha2","resourceVersion":"1388221816"}, "reason": "PodCreateError", "message": "Could not create Pod pods \"simple-template-example-9oz3h3r3-v3-setup-\" is forbidden: unable to validate against any security context constraint: [provider \"anyuid\": Forbidden: not usable by user or serviceaccount, provider \"pipelines-scc\": Forbidden: not usable by user or serviceaccount, provider \"tekton-pipelines-scc\": Forbidden: not usable by user or serviceaccount, provider restricted: .spec.securityContext.fsGroup: Invalid value: []int64{2002022-11-15T14:42:30.081602675Z 0}: 2000 is not an allowed group, spec.containers[0].securityContext.runAsUser: Invalid value: 2000: must be in the ranges: [1001880000,<PHONE_NUMBER>], provider \"nonroot\": Forbidden: not usable by user or serviceaccount, provider \"noobaa\": Forbidden: not usable by user or serviceaccount, provider \"noobaa-endpoint\": Forbidden: not usable by user or serviceaccount, provider \"scc-kubecost\": Forbidden: not usable by user or serviceaccount, provider \"sonardb-scc\": Forbidden: not usable by user or serviceaccount, provider \"hostmount-anyuid\": Forbidden: not usable by user or serviceaccount, provider \"log-collector-scc\": Forbidden: not usable by user or serviceaccount, provider \"apps-fluentd-scc\": Forbidden: not usable by user or serviceaccount, provider \"iam-scc\": Forbidden: not usable by user or serviceaccount, provider \"stakater-managed-openshift-apps-keycloak-scc\": Forbidden: not usable by user or serviceaccount, provider \"machine-api-termination-handler\": Forbidden: not usable by user or serviceaccount, provider \"hostnetwork\": Forbidden: not usable by user or serviceaccount, provider \"hostaccess\": Forbidden: not usable by user or serviceaccount, provider \"rook-ceph\": Forbidden: not usable by user or serviceaccount, provider \"node-exporter\": Forbidden: not usable by user or serviceaccount, provider \"privileged\": Forbidden: not usable by user or serviceaccount, provider \"rook-ceph-csi\": Forbidden: not usable by user or serviceaccount]"}

actually looking a bit closer at the error it is also flagged for the .spec.securityContext.fsGroup: 2000 but same concept described above applies here

Hi @callum-stakater I'm sorry your having this issue. I'm not familiar with OpenShift or the concept of SecurityContextConstraints. The usage for the security context with user/group 2000/2000 is actually just to solve an issue with multiple pods that share a PersistentVolume.
The id 2000 is a completely arbitrary number selection. However, the workflow has to guarantee that all the task pods get the same exact user/group. Those ids are used to mount the volume to make it readable and writeable. And the mounted volume contains all the data for a workflow to execute.
Is it just fsGroup 2000 that can't be used? What if the id was definable in the tf resource spec which gets applied to all task pods?

Not a problem, is the constant battle in the world of OpenShift administration :)
The number 2000 isn't important here, the issue is as OpenShift does "Secure by default" it doesn't allow setting the runAsUser or fsGroup without adding the additional scc configurations because if you are able to set any number you can set root (0), if you can set root and your container gets compromised its a bad day, so the default is for OpenShift to assign random IDs from a high range that each node has a pool of.
In this case though as the fsGroup/runAsUser is defined in the operator code , for a reason (shared volumes) its perfectly fine and a good practice for the vanilla kubernetes users so we will extend the deployment/chart and add the scc's we need

We are also running the operator on OKD/Openshift.
We usually handle this by adding the "anyuid" scc to the existing ClusterRole (via a kustomize patch without need of changing the upstream manifests).
´´´yaml

target:
group: rbac.authorization.k8s.io
version: v1
kind: ClusterRole
patch: |-

op: add
path: /rules/-
value:
apiGroups:

security.openshift.io
resources:
securitycontextconstraints
verbs:
use
resourceNames:
anyuid

or alternatively by adding the required User/Group directly to the namespace. This ommits to permit any SCC or create a new one as it fixes the range for users/groups to be used in that namespace.
Especially if its only 1 namespace where you run the operator and the workflows itself, this is probably the easiest solution and also doesn't contradict the SCC concept.

```yaml
kind: Namespace
apiVersion: v1
metadata:
  name: terraform-operator
  annotations:
    openshift.io/sa.scc.supplemental-groups: 2000/1
    openshift.io/sa.scc.uid-range: 2000/1

Perhaps this helps.

Ah nice, wasn’t aware of the annotation method, that does help
On Tue 13. 12. 2022 at 8:25, Daniel K @.***> wrote:

We are also running the operator on OKD/Openshift.
We usually handle this by adding the "anyuid" scc to the existing
ClusterRole (via a kustomize patch without need of changing the upstream
manifests).
´´´yaml

target:
group: rbac.authorization.k8s.io
version: v1
kind: ClusterRole
patch: |-

op: add
path: /rules/-
value:
apiGroups:

security.openshift.io
resources:
securitycontextconstraints
verbs:
use
resourceNames:
anyuid

or alternatively by adding the required User/Group directly to the namespace. This ommits to permit any SCC or create a new one as it fixes the range for users/groups to be used in that namespace.
Especially if its only 1 namespace where you run the operator and the workflows itself, this is probably the easiest solution and also doesn't contradict the SCC concept.

kind: Namespace

apiVersion: v1

metadata:

  name: terraform-operator

  annotations:

    openshift.io/sa.scc.supplemental-groups: 2000/1

    openshift.io/sa.scc.uid-range: 2000/1

Perhaps this helps.

—
Reply to this email directly, view it on GitHub
<https://github.com/isaaguilar/terraform-operator/issues/126#issuecomment-1347853865>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARVLIF5S6J6SMU5AUU32SX3WNAQGTANCNFSM6AAAAAASBA57CU>
.
You are receiving this because you were mentioned.Message ID:
<EMAIL_ADDRESS>


Broken URLs / automatic splits

Hello, first thank you for the great work you did.
I had the same Problem as some others with the brokens urls, for me it was the following Videos:
https://www.youtube.com/watch?v=kN-2mc561GY
https://www.youtube.com/watch?v=krPRThe7R4g
https://www.youtube.com/watch?v=JV0D-YkWHD8
https://www.youtube.com/watch?v=CyeT8rEQOpQ
https://www.youtube.com/watch?v=ezY7QrSWeWw
https://www.youtube.com/watch?v=GdyJqgxlSxs
https://www.youtube.com/watch?v=K1r3m5OrmB4
https://www.youtube.com/watch?v=gDjeTHGWX9c
https://www.youtube.com/watch?v=IFhWIS-Gwps
https://www.youtube.com/watch?v=HeK6zbf8I9Y
https://www.youtube.com/watch?v=6I7Dz0qF4NA
https://www.youtube.com/watch?v=RASKiMoxhOE
https://www.youtube.com/watch?v=pQdl0apLT70
https://www.youtube.com/watch?v=e3hXj7S3QyA
https://www.youtube.com/watch?v=RPNiqsdt6-w
https://www.youtube.com/watch?v=GJr8wKk8-YA
https://www.youtube.com/watch?v=UjkeNSr2dXQ
https://www.youtube.com/watch?v=Jt6_hinpKpk
https://www.youtube.com/watch?v=GDejMONyY4U
https://www.youtube.com/watch?v=IOoVklIZLMw
https://www.youtube.com/watch?v=eW9vWga9F5M
https://www.youtube.com/watch?v=Hfs4AlBmg70
https://www.youtube.com/watch?v=Z9K13eBUwJM
https://www.youtube.com/watch?v=ecRog9N1Qf0
https://www.youtube.com/watch?v=DkjTJcCXipY
https://www.youtube.com/watch?v=GkDXwJXjQx0
https://www.youtube.com/watch?v=uO2zGO5ydBc
https://www.youtube.com/watch?v=1u69z-wsDIc
https://www.youtube.com/watch?v=WWyeuvpHmfc
https://www.youtube.com/watch?v=kld-iYpdjyo
https://www.youtube.com/watch?v=xVfPBzDnI2Q
It would be great if you could add these files to your drive Folder or something.
I thought maybe about an automatic updater for the Videos so the splits/annotations can be created accordingly.
I would be willing to make this when you find this interesting.

Same here. It would be great if you can include these videos in the drive. Thank you!

first of all thanks for your work. there is no guarantee that all videos will be available in future, can you  share them in a drive or something else?


[Bug] When there is a formula in a row, the row cannot be deleted successfully.

Before you submit this issue, have you checked the following

[X] Is this really a problem?
[X] I have searched the Github Issues for similar issues, but did not find anything.

Affected packages and versions
v0.1.11
Reproduction link
https://univer.ai/zh-CN/playground
Expected behavior
可以正常删除
Actual behavior
删除不成功，删除另外一行的公式，前一行的公式又出现
System information
No response

Thank you for your question, we will fix it as soon as possible


Alphabetical Order

Hi @OzanKurt Thanks for the PR, but I don't think this actually matters. So I'll close this issue now


103 editprof

Added functionality of letting a user edit their profile attributes
fixed up some xmls to look consistent with design
fixed a sign up activity bug, should be consistent now

Can we add some field tags above each TextInput here to specify which Text input takes which fields.
For example:
First Name:
TextInput
Last Name
TextInput
Email:
TextInput

 Can we add some field tags above each TextInput here to specify which Text input takes which fields. For example: First Name: TextInput Last Name TextInput Email: TextInput

done, changes are live

here ya go

Fixed: Now not able to add multiple same QR codes in the wallet unless debug is true.


[BUG] invalid pyproject.toml config: data.project.entry-points

setuptools version
68.0.0
Python version
3.11.3
OS
Arch Linux
Additional environment information
This only happens using pyproject.toml.
If instead setup.cfg & setup.py is used, it works:
[metadata]
name = timmins-plugin
version = 0.1
description = A test
author = Anton Hvornum
author_email =<EMAIL_ADDRESS>long_description = a test
license = GPL
license_files =
    LICENSE
project_urls =
    Source = https://github.com/Torxed/timmins
    Documentation = https://timmins.readthedocs.io/
classifers =
    Programming Language :: Python :: 3
    Programming Language :: Python :: 3.11
    License :: OSI Approved :: GNU General Public License v3 (GPLv3)
    Operating System :: POSIX :: Linux

[options]
packages = find:
python_requires = >= 3.11

[options.entry_points]
timmins.display =
    excl = timmins_plugin_fancy:excl_display

Description
Unable to build plugins with pyproject.toml.
Expected behavior
The plugin to be built, installed and to be accessible with:
import importlib.metadata
for plugin_definition in importlib.metadata.entry_points().select(group='timmins.display'):
	plugin_entrypoint = plugin_definition.load()

How to Reproduce

Download timmins-plugin-fancy.zip (or follow https://setuptools.pypa.io/en/stable/userguide/entry_point.html#entry-points-for-plugins to create the same structure and files)
pip install . --break-system-packages

Output
Defaulting to user installation because normal site-packages is not writeable
WARNING: Skipping /usr/lib/python3.11/site-packages/uvicorn-0.21.1.dist-info due to invalid metadata entry 'name'
WARNING: Skipping /usr/lib/python3.11/site-packages/anyio-3.6.2.dist-info due to invalid metadata entry 'name'
WARNING: Skipping /usr/lib/python3.11/site-packages/starlette-0.26.1.dist-info due to invalid metadata entry 'name'
WARNING: Skipping /usr/lib/python3.11/site-packages/fastapi-0.95.1.dist-info due to invalid metadata entry 'name'
WARNING: Skipping /usr/lib/python3.11/site-packages/uvicorn-0.21.1.dist-info due to invalid metadata entry 'name'
WARNING: Skipping /usr/lib/python3.11/site-packages/anyio-3.6.2.dist-info due to invalid metadata entry 'name'
WARNING: Skipping /usr/lib/python3.11/site-packages/starlette-0.26.1.dist-info due to invalid metadata entry 'name'
WARNING: Skipping /usr/lib/python3.11/site-packages/fastapi-0.95.1.dist-info due to invalid metadata entry 'name'
Processing /home/anton/timmins-plugin-fancy
  Installing build dependencies ... done
  Getting requirements to build wheel ... error
  error: subprocess-exited-with-error

  × Getting requirements to build wheel did not run successfully.
  │ exit code: 1
  ╰─> [35 lines of output]
      configuration error:

      Traceback (most recent call last):
        File "/usr/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
          main()
        File "/usr/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/usr/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 118, in get_requires_for_build_wheel
          return hook(config_settings)
                 ^^^^^^^^^^^^^^^^^^^^^
        File "/tmp/pip-build-env-g8jj5rh6/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 341, in get_requires_for_build_wheel
          return self._get_build_requires(config_settings, requirements=['wheel'])
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/tmp/pip-build-env-g8jj5rh6/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 323, in _get_build_requires
          self.run_setup()
        File "/tmp/pip-build-env-g8jj5rh6/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 338, in run_setup
          exec(code, locals())
        File "<string>", line 1, in <module>
        File "/tmp/pip-build-env-g8jj5rh6/overlay/lib/python3.11/site-packages/setuptools/__init__.py", line 107, in setup
          return distutils.core.setup(**attrs)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/tmp/pip-build-env-g8jj5rh6/overlay/lib/python3.11/site-packages/setuptools/_distutils/core.py", line 159, in setup
          dist.parse_config_files()
        File "/tmp/pip-build-env-g8jj5rh6/overlay/lib/python3.11/site-packages/setuptools/dist.py", line 898, in parse_config_files
          pyprojecttoml.apply_configuration(self, filename, ignore_option_errors)
        File "/tmp/pip-build-env-g8jj5rh6/overlay/lib/python3.11/site-packages/setuptools/config/pyprojecttoml.py", line 66, in apply_configuration
          config = read_configuration(filepath, True, ignore_option_errors, dist)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/tmp/pip-build-env-g8jj5rh6/overlay/lib/python3.11/site-packages/setuptools/config/pyprojecttoml.py", line 128, in read_configuration
          validate(subset, filepath)
        File "/tmp/pip-build-env-g8jj5rh6/overlay/lib/python3.11/site-packages/setuptools/config/pyprojecttoml.py", line 55, in validate
          raise ValueError(f"{error}\n{summary}") from None
      ValueError: invalid pyproject.toml config: data.project.entry-points.
      configuration error:
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.

Here's the pyproject.toml:
[build-system]
requires = ["setuptools>=67.5"]
build-backend = "setuptools.build_meta"

[project]
name = "timmins"
dynamic = ["version", "entry-points"]
description = "A test"
authors = [
	{name = "Anton Hvornum", email =<EMAIL_ADDRESS>]
license = {text = "AGPL-1.0-or-later"}
readme = "README.md"
requires-python = ">=3.11"
dependencies = [
]

keywords = ["linux", "timmins"]

classifiers = [
	"Programming Language :: Python :: 3.11",
	"Operating System :: POSIX :: Linux",
]

[project.urls]
Home = "https://timmins.archlinux.life"
Documentation = "https://timmins.readthedocs.io/"
Source = "https://github.com/Torxed/timmins"

# https://setuptools.pypa.io/en/latest/userguide/entry_point.html#entry-points-syntax
# Note the quotes around timmins.display in order to escape the dot .
[project.entry-points."timmins.display"]
excl = "timmins_plugin_fancy:excl_display"

[options]
package_dir="src"
include_package_data = true

[options.packages.find]
where="src"

[options.package_data]
src = "*.py"

Hi @Torxed, please note that you cannot provide a value for project.entry-points and list it under project.dynamic at the same time. This is the reference: https://packaging.python.org/en/latest/specifications/declaring-project-metadata/#dynamic.

@abravalheri you are correct! Oversight, I just went plainly on the error message stating it was a invalid config of value entry-points.
Thank you!


Gateway profile does not start up due to an osgi issue

Description
The server hangs with the following error when starting the pack with Gateway profile.
[2022-11-02 21:11:41,376]  INFO - UserStoreMgtDSComponent Carbon UserStoreMgtDSComponent activated successfully.
[2022-11-02 21:12:41,701]  WARN - CarbonServerManager Carbon initialization is delayed due to the following unsatisfied items:
[2022-11-02 21:12:41,701]  WARN - CarbonServerManager Waiting for required OSGi Service: org.wso2.carbon.event.publisher.core.EventPublisherService

Note : org.wso2.carbon.event.publisher.core component is in an active state.
Steps to Reproduce

Do profile optimization for the gateway profile.
Start the pack with gateway profile.

Affected Component
APIM
Version
APIM 4.2.0 M1 testing pack
Environment Details (with versions)
No response
Relevant Log Output
No response
Related Issues
No response
Suggested Labels
No response

Please find the diff of part of the fix attached here.
gwFixDiff.txt
The gateway profile still shows the following error now when starting.
[2022-11-02 21:46:53,592]  WARN - StartupFinalizerServiceComponent Waiting for required OSGi services: org.wso2.carbon.identity.application.authentication.framework.ApplicationAuthenticationService,org.wso2.carbon.identity.oauth.OAuthAdminServiceImpl,

The following diff will fix the issue.
finalDiff.txt

This issue is fixed with the commit https://github.com/wasuradananjith/product-apim/commit/a6a1dc532695791a820004462cf652e3adf076ab


⚠ SceneTime has degraded performance

In 7c9c4e8, SceneTime ($ST) experienced degraded performance:

HTTP code: 200
Response time: 1427 ms

Resolved: SceneTime performance has improved in fc0517f after 6 minutes.


Switch tags to key/value system

Tags should be added as key/value pairs instead of an array of strings to more closely match the systems they are being send to like Sentry.
Console logging output should be: [key1:value1|key2:value2]

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

npm package (@latest dist-tag)
GitHub release
v1.0.4

Your semantic-release bot :package::rocket:


Fix bug where BPs were showing up multiple times in VS

This bug is because gdb may bind the bp in a different location in the
symbol file and the address of where that is loaded in memory changes
before and after the program is launched, causing us to send multiple
breakpoint binds without an unbind for the same breakpoint id.
Fixed so we will:
A) Clear the SourceLineCache right before we start debuggee launch in a
launch scenario
B) Requery gdb for the source line to address mappings
C) use that updated SourceLineCache to determine the correct line number
for the breakpoint
D) Send a BreakpointUnbound() message with the status of Rebind for the
old breakpoint to VS to update the UI.

@richardstanton who did the original PR


Microsoft.ContainerService and Microsoft.ServiceFabric api versions

In an https://docs.microsoft.com/en-us/azure/azure-stack/user/azure-stack-develop-templates#api-versioning section we have a list of ProviderNamespaces, but looks like it's complete.
here https://docs.microsoft.com/en-us/azure/azure-stack/user/azure-stack-solution-template-kubernetes-deploy and https://docs.microsoft.com/en-us/azure/azure-stack/user/azure-stack-solution-template-service-fabric-cluster I can find a kubernates/service fabric services, probably managed by Microsoft.ContainerService and Microsoft.ServiceFabric
Could you share the API version for both namespaces?
Could you also specify a list of resources managed by listed providers for Azure Stack?

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

ID: c633e983-0f74-d2e6-ba26-9c0ba555e8f0
Version Independent ID: ef5907bb-7f21-7cda-bc9a-3cc60e5390b2
Content: Develop templates for Azure Stack
Content Source: articles/azure-stack/user/azure-stack-develop-templates.md
Service: azure-stack
GitHub Login: @sethmanheim
Microsoft Alias: sethm

Thanks for the feedback! I have assigned the issue to the content author to evaluate and update as appropriate.

Not relevant, because I finally understood that both services are essentially VMSS with pre installed software. Initially I thought that we have respective providers on Azure Stack.


Added a stylize() convenience method.

I'm using this in a custom argparse.Formatter() and I keep having to wrap statements in the ANSI shortcuts, so I just wrote a method that auto-resets after the string. You can use an arbitrary number of args to list styles or mix and match as desired.

Thanks again @jwarner112 !!!


Update README.md

Please add under Attributes:
Is via repeater
Alarm
Armed
Stay away
Maybe add also some images that users know what can expect from integration to avoid not working confusions...
Maybe one image for PIR sensor and one image for magnetic sensor...
Magnetic contact:

PIR:

After mering #7 system device screenshot will be added too


qa_crowbarsetup.sh: onadmin_rebootcloud fixes

If the onadmin_rebootcloud function is invoked before tests
have been run, the testvm machine isn't present.
After reboot, the manila-service VM also needs to be started,
if it was shutdown during reboot.

Tested with: https://ci.suse.de/blue/organizations/jenkins/cloud-crowbar8-job-ha-x86_64/detail/cloud-crowbar8-job-ha-x86_64/30/pipeline/
The job failed because it takes the second controller too much time to reboot.


SwingPanel requires a fixed content size

Describe the bug
If you don't set a fixed size on SwingPanel and its contents, the component will have an arbitrary size or cause crashes in the Compose Node layout pass.
Affected platforms
Select one of the platforms below:

Desktop

Versions

Kotlin version*: 1.7.20
Compose Multiplatform version*: 1.2.1
OS version(s)* (required for Desktop and iOS issues): macOS/Swing
OS architecture (x86 or arm64): arm64 but irrelevant
JDK (for desktop issues): 17

To Reproduce
Steps and/or the code snippet to reproduce the behavior:

Create an IJ plugin project (using the template in the samples in this repo)
Create a LazyColumn and add items to it
Make those items be a SwingPanel that contains a non-size-constrained JComponent
Observe the items get sized incorrectly

Expected behavior
Components shouldn't have to be fixed size, as it's a big limitation. You don't always know what size you need in advance.

I have a repro here: https://github.com/rock3r/cfd-repros/blob/main/src/main/kotlin/ComposePanelFixedSize.kt
The ComposePanel's content doesn't show up unless I set a fixed height on it. If I use heightIn(minH, maxH) the panel will always be sized to minH, which is also wrong. I suspect this to be related to #4556, but it's slightly different.
The original crash I was seeing happened because I used an IntelliJ's EditorTextField inside it, which is scrollable, and I was in a LazyColumn. In the simple repro, there is no crash, but it still does not work as intended.

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.


Bug: Files app hangs during OneDrive file download.

Description
When I try to access specific files on OneDrive using the Files app, the app often freezes and becomes unresponsive. Sometimes, downloading these files either doesn't start at all and stays at zero percent completion, or at least progresses extremely slowly.

Steps To Reproduce

Find a file larger than 15-20 megabytes on OneDrive
try to download it by opening it
Look for download notification from Onedrive... "Files is trying to download..."
wait for Files to freeze

Files Version
<IP_ADDRESS>
Windows Version
10.0.26100.2314
User ID
d40ec6d5-d32d-438f-9f5b-be1a31344696
Log File
debug.log

Can you check if this happens with File Explorer as well?

@yaira2 -- Yes, I have noticed OneDrive is a little slow and some files take a little longer to download. However, Explorer does not freeze and become unusable. The file simply downloads in the background.


Improve control flow loop analysis logic

This PR cleans up and adds more comments to the logic for control flow type analysis of loops. Caches are now associated with the FlowLabel nodes themselves (and not their antecedent nodes) and the odd cases previously returned undefined no longer occur.

@mhegazy @vladima Give it a look when you get a chance.

:+1:


Cross-Platform Support

Since Duality is using OpenTK, it should in theory be possible to run it with Mono / on different platforms than Windows
Some dependencies might be problematic indeed and need to be removed or replaced. OggVorbis might be an example
While making the engine itself multiplatform-ready seems like an achievable goal, the editor will probably need a lot more work.
Will need a testing environment / system and someone with Mono experience

This issue has been superseded by the OpenTK Abstraction Layer and making Duality a PCL. When both of them are in place, it should be possible to port Duality by simply creating a new platform backend plugin and, depending on the platform, a new launcher.
With the entire platform backend in a plugin and Duality and its plugins being PCLs, it would in theory be possible to reach just about any platform that is supported by the PCL profile - all Desktop ones (DotNet and Mono), as well as some of the Xamarin ones - as long as someone creates the platform backend plugin and potentially launcher.
Closing this, moving discussion to the v2.0 milestone.


HOA-290 validation messages signals

a few things that we need to take a closer look at:

purpose of materialErrorMatcher property
purpose of _clearErrors method
apiErrorMessages

name apiErrorMessages should rather be explicitErrorMessages
should we even modify formControl by setting errors that are coming from apiErrorMessages?


pipeline table.insert #2 argument out of bound

in redis.lua file, pipeline function Line 516
request is {}, empty, so #request get 0, it's a bad argument for table.insert

But the call to 'client.network.write' adds data to requests
If I'm not mistaken:
local reply = cmd(client, ...)  <--- this will end up inserting data in 'requests' table
-- so here #requests can't be zero
table_insert(parsers, #requests, reply.parser)
return reply

sounds reasonable, but in particular situation, request is empty, that's a sure thing. and lua don't allow #2 argument to be 0, so an empty table can't use table.insert with #2 argument, which is a strange design i think.
if Line 516 isn't wrong, then the responsibility falls to the code before it by not catching the error in advance.

Do you have a test case to reproduce the error?

Not sure I understand what the issue is, any tiny snippet of code to reproduce it?

I'm seeing this issue as well.
redis-lua
   2.0.4-1 (installed) - /usr/local/lib/luarocks/rocks

local redis = require("redis")
local r = redis.connect()

r:pipeline(function(pipe)
    pipe:zadd("FOO", 1, "BAR")
    pipe:zadd("FOO", 2, "BAZ")
end)

lua: /usr/local/share/lua/5.2/redis.lua:802: /usr/local/share/lua/5.2/redis.lua:471: bad argument #2 to 'table_insert' (position out of bounds)
stack traceback:
	[C]: in function 'error'
	/usr/local/share/lua/5.2/redis.lua:802: in function 'error'
	/usr/local/share/lua/5.2/redis.lua:480: in function 'pipeline'
	test.lua:5: in main chunk
	[C]: in ?

@benwilber Did you find a fix for this?

I got the same problem, but it seems that it comes from lua version:
in 5.1:
t = {}
table.insert(t, 1, nil)
print(#t) --> 0, but
table.insert(t, 2, nil) --> no error

in 5.2 and 5.3:
t = {}
table.insert(t, 1, nil)
print(#t) --> 0
table.insert(t, 2, nil) --> ERROR: bad argument #2 to 'insert' (position out of bounds)

corrected for me by changing
in pipeline function:
table_insert(parsers, #requests, reply.parser)

by
parsers[#requests] = reply.parser

I'm working on a more complete patch and I submit a pull request

:+1: well done @cedroyer! This seems to affect Lua >= 5.2 indeed.

I think the correct fix is to change:
table_insert(parsers, #requests, reply.parser)

to:
table_insert(parsers, reply.parser)

This keeps the requests and parsers tables parallel, i.e. for requests[n] the correct parser is parsers[n].

I disagree, if you use:
table_insert(parsers, reply.parser)
and reply.parser is nil (and that seems to be the case), your table won't change and the next parser you add in parsers will be insert at the wrong place (n-1), that's why table_insert(parsers, #requests, reply.parser) was used in the library.
use parsers[#requests] = reply.parser works for me with lua 5.3 and I think it also works for lua 5.1 and 5.2.


Use it as Month selector only

Hi, could be possible to use this to select the year and the month only?
Sometimes it would be very useful for accountancy.
I imagine it like just like the Select year first demo but when i select the month wouldn't get the day selection screen but it closes and gives me the date with the first day of the selected month.
Thanks

I would like this as well.  Also being able to select a range of months would be great.

@sun2rise , @adhesivewombat
I hope this PR ( https://github.com/clauderic/react-infinite-calendar/pull/189) can make you happy.

tnx @EthanShao this is sooooo good!


removed pro tip on applying for twitter cards

apparently this is no longer requried
documentation https://dev.twitter.com/cards/getting-started does not say anything about applying for this, and some googling indicates this may no longer be required

Looks to be the case.

Run your URL against the validator tool to test. If you are working with a Player Card, request approval for whitelisting. All other Cards do not need whitelisting.


Tegra out of range

maverick.dev.2018-01-20-21-22.log

I think this should be fixed by previous maverick_desktop updates, please reopen if still an issue


malformed document. First element should be

The top-level README.md is dynamically generated so the readme changes should actually go here: https://github.com/nativescript-community/universal-links/blob/master/packages/universal-links/blueprint.md
Could you put the changes there instead and then run npm i and npm run readme to generate the new readme for the top-level?


Improve error handling

Right now errors returned by the recv method are lost because we do this :
go c.recv()
and recv used to return an error.
I changed it a little already, but I need to refactor the code to use a channel for the errors.

Fixed by #135


Per-endpoint configurable certificate_authority

Purpose
To securely connect to providers using self-signed TLS certificates, it should be possible to configure ManageIQ to trust specific CA(s).
Currently this generally requires adding them to the system CA bundle on the MIQ machine; Openstack looks at settings ssl.ssl_ca_file and ssl.ssl_ca_path but you still have to place the file(s) on the machine first.
This PR adds a way to to store the CA in the DB + a bit of support logic + REST API.
It remains up to specific providers to use it (exact use will vary by network lib but something like passing :ssl_cert_store => endpoint.ssl_cert_store).

There can be a secondary use case of "pinning a CA": my provider has a globally trusted cert from a TopSecureCA™, and I want to trust only them, not the standard bundle of over 150 CAs.
I'm assuming here that whenever you specify a custom CA, you no longer want to trust the system bundle, so this case is also covered.

Rationale: Global / Per Provider / Per EMS / Per Endpoint ?

A global pool of trusted CAs would I think be less secure (I trust Alice to administrate provider A and Bob to administrate provider B, but I don't want Bob to be able to impersonate A) and pragmatically inconvenient to manage — hard to know which certs can be removed when.

Per Endpoint may be best since different endpoints could have different host, port and maybe certs.  YAGNI?

Per Provider or EMS can be OK too, especially if we allow multiple certs.

=> I've chosen per-Endpoint because it logically belongs with verify_ssl and security_protocol columns that Endpoint already has.
Rationale: text column holding PEM cert(s)
A few endpoints might end up storing the same cert, but seemed to me much simpler than doing a new table with 1:many association...
I'm storing certs in PEM format — essentially the ASN-1 binary encoded in base64.  Opaque, but it's what essentially all network libs take.
The parsing logic supports multiple concatenated PEM certs.  There is no strong need, but it's a common thing people do with trust roots, so I figured it's easier to make it work than forbid...
REST API
Same as other endpoint attributes (see #13454):

Returned in endpoints (when non-nil).
For default endpoint can be set as a top-level attribute.
Can be set on endpoints inside connection_configurations.

@miq-bot add-label enhancement, providers, sql migration, api
cc @simon3z @blomquisg @Fryguy

Aside from my one comment on the testing of multiple certs, LGTM :+1:

Interesting, this now fails a migration spec from #13324:
https://travis-ci.org/ManageIQ/manageiq/jobs/193401493
at https://github.com/ManageIQ/manageiq/pull/13324/files#diff-230601d4d50b88deb0491bee98291b9dR8
Apparently using FactoryGirl uses app/models/, which ends up running the validation I added here, which can't work before the (later) migration here created the certificate_authority column.
@moolitayer is working on a fix.

@Fryguy my only comment here is that I would expect that a CA per provider would be enough (actually you generally have one CA per company... so even per-provider would be already overkill).
If this approach is simpler, more flexible and requires only some magic in the UI to copy the same certificate in all the endpoints for the vast majority of the cases... then I'm OK with it. LGTM :+1:

There will now be 3 related fields: (security_protocol, verify_ssl, certificate_authority).
Code will typically look at some combination of them, so I think it's simplest to store them together and set them together.

The exact semantics varies by provider.  Some check security_protocol, some check verify_ssl, some neither; I think SCVMM uses security_protocol == "ssl" sanely but several use "ssl-with-validation" for that and security_protocol == "ssl" means insecure...
From UI perspective it'll be natural to add CA field that only shows for a specific selection in security_protocol dropdown.

@tzumainn @aufi it's something you could be interested in guys. Defining cert in the UI, rather than Settings.ssl.ssl_ca_file and Settings.ssl.ssl_ca_path


Critical: Fix DDP barriers

What does this PR do?

This PR aims at fixing an issue with DDP barrier. Indeed, in the current develop version of SpeechBrain this current code snippet doesn't work:
  from speechbrain.utils.distributed import get_rank, rank_prefixed_message, is_distributed_initialized, if_main_process, ddp_barrier

  logger.info(rank_prefixed_message(f"main={if_main_process()}"), main_process_only=False)

  if if_main_process():
      import time
      time.sleep(10)
  logger.info(rank_prefixed_message("barrier 1 reached"), main_process_only=False)
  ddp_barrier()
  if if_main_process():
      import time
      time.sleep(10)
  logger.info(rank_prefixed_message("barrier 2 reached"), main_process_only=False)
  ddp_barrier()

  logger.info(rank_prefixed_message("finished"), main_process_only=False)

  torch.distributed.destroy_process_group()

while it should! In the current dev version, the rank1 is reaching the end of the program and doesn't wait in the second DDP barrier. It does wait in the first for the main proc, but does not in the second. I went into debugging mode and found that the LOCAL_RANK of each proc is 0, which happen as we do not instantiate Brain() and thus the cuda device stream is never set to the actual LOCAL_RANK. I found that explicitly setting torch.cuda.set_device(device) in ddp_init_group was solving the issue. When you call torch.cuda.current_device() you get the right LOCAL_RANK value (which was not the case as it was just returning the LOCAL_RANK 0).
Another example with run_on_main:
from speechbrain.utils.distributed import get_rank, rank_prefixed_message, is_distributed_initialized, if_main_process, ddp_barrier

def bar():
    import time
    time.sleep(10)
    logger.info(rank_prefixed_message("SB is so good."), main_process_only=False)

def foo():
    bar()

run_on_main(
    foo
)
run_on_main(
    foo
)
logger.info(rank_prefixed_message("end."), main_process_only=False)

The second rank is rightly awaiting for foo to get executed and wait for the DDP barriers.

Before submitting

[ ] Did you read the contributor guideline?
[ ] Did you make sure your PR does only one thing, instead of bundling different changes together?
[ ] Did you make sure to update the documentation with your changes? (if necessary)
[ ] Did you write any new necessary tests? (not for typos and docs)
[ ] Did you verify new and existing tests pass locally with your changes?
[ ] Did you list all the breaking changes introduced by this pull request?
[ ] Does your code adhere to project-specific code style and conventions?

PR review

Reviewer checklist

[ ] Is this pull request ready for review? (if not, please submit in draft mode)
[ ] Check that all items from Before submitting are resolved
[ ] Make sure the title is self-explanatory and the description concisely explains the PR
[ ] Add labels and milestones (and optionally projects) to the PR so it can be classified
[ ] Confirm that the changes adhere to compatibility requirements (e.g., Python version, platform)
[ ] Review the self-review checklist to ensure the code is ready for review

I thought LOCAL_RANK was supposed to be set by torchrun is that not the case?

I thought LOCAL_RANK was supposed to be set by torchrun is that not the case?

It is but at the cuda level, it seems that torch barrier is using torch.cuda.device to synchronize processes but since the variable is never set properly (meaning that LOCAL_RANK is not used here) then pytorch fails to synchronize barriers. I believe it might be only located to NCCL backend.

torch barrier is using torch.cuda

That is a very odd choice by the PyTorch team....


NeverEnding not behaving

Hi Dirk,
I've just implemented your jQuery slider, nice work.
One problem with the neverEnding setting. I've implemented a previous arrow and a next arrow and I have 3 slides. Every time you click the next arrow and wait till the sliding transition has ended before you click another time on the next arrow, it goes to the next slides perfectly and loops "never ending".
However, if you click the next arrow, and during the slide transition you click another time, then at the final slide the slides slide back (sliding left instead of right) to the first slide as if the neverEnding setting was set to false. It doesn't loop anymore.
My code:
$('#slider').simpleSlider({
slides: '.slide',
slideOnInterval: false,
magneticSwipe: false,
slideTracker: false,
//neverEnding: false
});
slider = $('#slider').data("simpleslider");
$('.control_next').click( function() {
slider.nextSlide(); // Go to the next slide
});
$('.control_prev').click( function() {
slider.prevSlide(); // Go to the previous slide
});

This indeed is a bug at the moment. The sliding mechanism clones the first and last slide and adds it to the end and beginning of the slide container. When you call the next slide it will go to all slides + 1 (which visualizes the first), but when you call the next slide again it will go to slide 2.
I will look into this as soon as possible, but at the moment the only workaround would be to prevent sliding when the slider is sliding.
In case of your implementation that would like something like this:
var sliding = false;

$("#slider").on("beforeSliding", function(event){
    sliding = true;
});

$("#slider").on("afterSliding", function(event){
    sliding = false;
});

$('.control_prev').click( function() {
    if(!sliding){
        slider.prevSlide(); // Go to the previous slide
    }
});

Thanks for the quick reply and the workaround.
One other quirky thing is when you have only 2 slides and neverEnding: true. Clicking the next button makes the slider go left instead of right.

Funky, I will look into it.


[Jira] Can't find any issues

Extension
https://www.raycast.com/raycast/jira
Description

Error:
TypeError: Cannot read properties of undefined (reading 'name')

Xy:IssueListItem.tsx:15:40
    at ray-list-section
cs:index.js:23:15636
Or:index.js:5:5486

Steps To Reproduce

Open Raycast
Install Jira extension
Authorise
Search for an issue

Current Behaviour
No response
Expected Behaviour
No response

They are three objects that can have the name property in IssueListItem: status, issuetype, and priority. I assumed these fields would always be defined since a Jira issue should always have a status, a type, or a priority 🤔


url escape db passwords

Secure Coding Practices Checklist GitHub Link

https://github.com/RedHatInsights/secure-coding-checklist

Secure Coding Checklist

[x] Input Validation
[x] Output Encoding
[x] Authentication and Password Management
[x] Session Management
[x] Access Control
[x] Cryptographic Practices
[x] Error Handling and Logging
[x] Data Protection
[x] Communication Security
[x] System Configuration
[x] Database Security
[x] File Management
[x] Memory Management
[x] General Coding Practices

Codecov Report

Merging #50 (bb463ae) into master (4d813e2) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master      #50   +/-   ##
=======================================
  Coverage   52.71%   52.71%
=======================================
  Files          11       11
  Lines         607      607
=======================================
  Hits          320      320
  Misses        267      267
  Partials       20       20

Flag
Coverage Δ

unittests
52.71% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

base/utils/database.go
0.00% <0.00%> (ø)

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 4d813e2...bb463ae. Read the comment docs.


Create my dataset but the error is “WARNING: You must download the raw data for the ‘my_own' dataset.”

dear @drprojects ,
I am having problems creating my own dataset 'scaffolds' and running train.py. I have read the create your own dataset section and created 'scaffolds' in the format of the ‘scannet’ dataset. Created /src/datasets/scaffolds.py & scaffolds_config.py, /src/datamodules/scaffolds.py, configs/datamodule/ semantic/scaffolds.yaml, configs/experiment/semantic/scaffolds_11g.yaml.
A new folder /data/scaffolds/raw/ is created to save the ‘scaffolds’ dataset in it, the ‘train&val’ part is saved in the /train folder as the scannet dataset is all saved in /scan folder, and the ‘test’ part is saved in the /test folder, the content of which is that the sem_label of each point has already been matched with the xyzrgb matching in the form of a sceneXXXX_point_clouds_all.npz file as follows:
{self.root = ''/ssd1/gaozy/Code/superpoint_transformer/data/scaffolds''}/
        └── raw/
            ├── train/
            │   └── sceneXXXX_point_clouds_all.npz
            └── test/
                └── sceneXXXX_point_clouds_all.npz

However, when I try to run train.py, it will get the error as follow.
I looked at my /data/scaffolds folder, which does have my own data already saved in it following the above structure, and I'm not sure what went wrong to cause it to jump straight to when the code is run:
# /superpoint_transformer/src/datasets/base.py--line599
def download(self):
        self.download_warning()
        self.download_dataset()

By the way, /ssd1/gaozy/Code/superpoint_transformer/data/scannet also exists correctly and runs successfully.
Thank you in advance for your help and support.
**_The ERROR Output_**
[2024-06-23 13:06:57,681][__main__][INFO] - Instantiating datamodule <src.datamodules.scaffolds.ScaffoldsDataModule>
[2024-06-23 13:06:58,603][__main__][INFO] - Instantiating model <src.models.semantic.SemanticSegmentationModule>
[2024-06-23 13:06:58,981][__main__][INFO] - Instantiating callbacks...
[2024-06-23 13:06:58,982][src.utils.utils][INFO] - Instantiating callback <pytorch_lightning.callbacks.ModelCheckpoint>
[2024-06-23 13:06:58,991][src.utils.utils][INFO] - Instantiating callback <pytorch_lightning.callbacks.EarlyStopping>
[2024-06-23 13:06:58,995][src.utils.utils][INFO] - Instantiating callback <pytorch_lightning.callbacks.RichModelSummary>
[2024-06-23 13:06:58,997][src.utils.utils][INFO] - Instantiating callback <pytorch_lightning.callbacks.RichProgressBar>
[2024-06-23 13:06:58,998][src.utils.utils][INFO] - Instantiating callback <pytorch_lightning.callbacks.LearningRateMonitor>
[2024-06-23 13:06:59,000][src.utils.utils][INFO] - Instantiating callback <pytorch_lightning.callbacks.GradientAccumulationScheduler>
[2024-06-23 13:06:59,002][__main__][INFO] - Instantiating loggers...
[2024-06-23 13:06:59,002][src.utils.utils][INFO] - Instantiating logger <pytorch_lightning.loggers.wandb.WandbLogger>
[2024-06-23 13:06:59,166][__main__][INFO] - Instantiating trainer <pytorch_lightning.Trainer>
Trainer already configured with model summary callbacks: [<class 'pytorch_lightning.callbacks.rich_model_summary.RichModelSummary'>]. Skipping setting a default `ModelSummary` callback.
GPU available: True (cuda), used: True
TPU available: False, using: 0 TPU cores
HPU available: False, using: 0 HPUs
[2024-06-23 13:07:07,673][__main__][INFO] - Logging hyperparameters!
/ssd1/gaozy/.vscode-server/extensions/ms-python.debugpy-2024.0.0-linux-x64/bundled/libs/debugpy/adapter/../../debugpy/launcher/../../debugpy/../debugpy/_vendored/force_pydevd.py:18: UserWarning: incompatible copy of pydevd already imported:
 /ssd1/gaozy/miniconda3/envs/spt/lib/python3.8/site-packages/pydevd_plugins/__init__.py
  /ssd1/gaozy/miniconda3/envs/spt/lib/python3.8/site-packages/pydevd_plugins/extensions/__init__.py
  /ssd1/gaozy/miniconda3/envs/spt/lib/python3.8/site-packages/pydevd_plugins/extensions/pydevd_plugin_omegaconf.py
  warnings.warn(msg + ':\n {}'.format('\n  '.join(_unvendored)))
wandb: Currently logged in as:<PHONE_NUMBER>6 (gaozeyu). Use `wandb login --relogin` to force relogin
wandb: Tracking run with wandb version 0.17.2
wandb: Run data is saved locally in /ssd1/gaozy/Code/superpoint_transformer/logs/train/runs/2024-06-23_13-06-54/wandb/run-20240623_130711-w7sr5nc2
wandb: Run `wandb offline` to turn off syncing.
wandb: Syncing run SPT-128
wandb: ⭐ View project at https://wandb.ai/gaozeyu/spt_scaffolds
wandb: 🚀 View run at https://wandb.ai/gaozeyu/spt_scaffolds/runs/w7sr5nc2
[2024-06-23 13:07:18,591][__main__][INFO] - Starting training!
**[2024-06-23 13:07:18,602][src.datasets.base][INFO] - WARNING: You must download the raw data for the Scaffolds dataset.
[2024-06-23 13:07:18,603][src.datasets.base][INFO] - Files must be organized in the following structure:
[2024-06-23 13:07:18,603][src.datasets.base][INFO] -
    /ssd1/gaozy/Code/superpoint_transformer/data/scaffolds/
        └── raw/
            ├── train/
            │   └── sceneXXXX_point_clouds_all.npz
            └── test/
                └── sceneXXXX_point_clouds_all.npz

[2024-06-23 13:07:18,604][src.datasets.base][INFO] -
[2024-06-23 13:07:18,604][src.datasets.scaffolds][ERROR] -
Scaffolds does not support automatic download.
Please place the dataset files in the correct structure in your /ssd1/gaozy/Code/superpoint_transformer/data/scaffolds/' directory and re-run.
The dataset must be organized into the following structure:

    /ssd1/gaozy/Code/superpoint_transformer/data/scaffolds/
        └── raw/
            ├── train/
            │   └── sceneXXXX_point_clouds_all.npz
            └── test/
                └── sceneXXXX_point_clouds_all.npz**

[2024-06-23 13:07:18,621][src.utils.utils][INFO] - Closing loggers...
[2024-06-23 13:07:18,621][src.utils.utils][INFO] - Closing wandb!
wandb:
wandb: 🚀 View run SPT-128 at: https://wandb.ai/gaozeyu/spt_scaffolds/runs/w7sr5nc2
wandb: ⭐ View project at: https://wandb.ai/gaozeyu/spt_scaffolds
wandb: Synced 6 W&B file(s), 0 media file(s), 0 artifact file(s) and 0 other file(s)
wandb: Find logs at: ./logs/train/runs/2024-06-23_13-06-54/wandb/run-20240623_130711-w7sr5nc2/logs
wandb: WARNING The new W&B backend becomes opt-out in version 0.18.0; try it out with `wandb.require("core")`! See https://wandb.me/wandb-core for more information.

Hi @zeyu659
This is probably linked to how you have configured your all_base_cloud_ids.
Our BaseDataset class inherits from PyG's Dataset class. As explained here. The download() method is called when the Datatest could not find all the raw_file_names in the raw/ directory.
Have a closer look at the PyG documentation and our BaseDataset code to understand how the raw_dir and processed_dir work. In particular, for your situation, I would suggest you have a closer look at how the following work:

all_base_cloud_ids
all_cloud_ids
cloud_ids
raw_file_names


Why isn't their a generic version of PropertyValidator

Hey
Great library!
I'm trying to understand why their is no generic version of PropertyValidator as in PropertyValidator<T>.
Why are we forced to do the following as an example:
var propertyValue = context.PropertyValue as string;
When rules are define as in
RuleFor(c => c.PropertyName).SetValidator(propertyValidaterHere); there's no compile checking to ensure the Property Name matches the type the validator was meant to validate
Is their any way to work around this?

Hi,
PropertyValidator is part of the internal API, which does not use generics to allow additonal flexibility especially when working with reflection, or performing advanced operations where type info may not be available. The public API (AbstractValidator and the RuleFor fluent interface) then wraps the internal API and provides type safety checks and strong typing via generics.
Typically, PropertyValidator is only used for advanced scenarios which can't be accomplished via the public API. For the vast majority of cases using PropertyValidator is unnecessary and the same result can be achieved in a much simpler way by using either extension methods or the Custom method. Both of these approaches are strongly typed with generics. If possible, I'd suggest using these and not PropertyValidator unless you really need it. More info on the different ways of performing custom valdiation can be found at https://fluentvalidation.net/custom-validators (specifically https://fluentvalidation.net/custom-validators#writing-a-custom-validator)

Thanks for the quick and detailed response.
What are my options if I want this "Property Validator" to be encapsulated in a separate class and don't want to use an extension method because I want my IOC to inject the relevant external dependencies needed to perform the validation.
I could just derive from AbstractValidator<T> and have a single rule in it, I just thought it was more appropriate to use a PropertyValidator
Let me give an example
my main validator is
public class MainValidator: AbstractValidator<User>
{
 public MainValidator(SpecificPropertyValidator specificPropertyValidator)
 {
 RuleFor(c=>c.PropertyName).SetValidator(specificPropertyValidator);
 }
}
And the specific property validator is
public class SpecificPropertyValidator : AbstractValidator<PropertyType>
{
 public SpecificPropertyValidator(IOC External Dependencies Here)
 {
 RuleFor(c=>c.PropertyValue).Custom()
// Seems funny because I only want this to validate a specific property
}
}

I could just derive from AbstractValidator and have a single rule in it, I just thought it was more appropriate to use a PropertyValidator

I think it depends on the type of logic you're going to put in there. If you want to write it in a more procedural style, then stick with PropertyValidator. If you want to make use of any of FV's other rules or write in a more composable style, then use an AbstractValidator derivative like in your example. I don't think there's really a right/wrong way in this particular situation.


Add table of contents to README

(Issue #82)
Long file, helpful to be able to jump to specific sections

There was Travis related update since my pull request.  I will make another pull request regarding issue #82 see if the new one will pass the check


Supplying tandems to RichText links

In https://github.com/phetsims/scenery/issues/1078, I added Tandem.OPT_OUT for links in RichText listeners, however it's likely we'll want to provide tandems for those input listeners.
How should  links in RichText be provided with tandems, or should they?  would take the RichText's tandem and createTandem( 'foo' ) it?
Thoughts?

 seems most scalable to me, if it is not too difficult.

 seems most scalable to me, if it is not too difficult.

Also... is the absence of that a good way to opt-out of a tandem? It seems like we'd want the default to be "you have to supply a tandem"?

Also... is the absence of that a good way to opt-out of a tandem? It seems like we'd want the default to be "you have to supply a tandem"?

How about  and an assertion check that the tandemName must end with *InputListener if not 'null'?  Or maybe it should be *FireListener, I'm not sure.

I think that the structure should be link.fireListener, so tandemName should either assert that it ends in Link, or Link should get tacked on (I think an assertion is most idiomatic).
Then we can have a consistent api for the fireListener.
Another idea. We could just use numbers, like myText.link1, and have a private group tandem. That may be easier, and nice to not expose tandemName in translated strings etc.
I also want to add a concern for dynamic elements. I don't know the inner workings of RichText.rebuildRichText, but in my mind, the worst case scenario is that every time the text of the richText changes (text that doesn't apply to the link), then the link would need to get recreated too. Is RichText smarter than that? If not, we may need to work on another phet-io feature in which disposing and recreating the same phetioID in the same frame is allowed (@samreid I can't find that issue, can you?).
Likely things are smarter than my above paragraph, but we should confirm, Take this example. . .

myText = new RichText( 'Hello Friend' ); // Instrumented FireListener created here

myText.text = 'Hello Friendly Friendly Giant'; // Does it know that that a tag is the same? Or Will it tear it down and recreate?

I think that the structure should be link.fireListener, so tandemName should either assert that it ends in Link, or Link should get tacked on (I think an assertion is most idiomatic).
Then we can have a consistent api for the fireListener.
Another idea. We could just use numbers, like myText.link1, and have a private group tandem. That may be easier, and nice to not expose tandemName in translated strings etc.
I also want to add a concern for dynamic elements. I don't know the inner workings of RichText.rebuildRichText, but in my mind, the worst case scenario is that every time the text of the richText changes (text that doesn't apply to the link), then the link would need to get recreated too. Is RichText smarter than that? If not, we may need to work on another phet-io feature in which disposing and recreating the same phetioID in the same frame is allowed (@samreid I can't find that issue, can you?).
Likely things are smarter than my above paragraph, but we should confirm, Take this example. . .

myText = new RichText( 'Hello Friend' ); // Instrumented FireListener created here

myText.text = 'Hello Friendly Friendly Giant'; // Does it know that that a tag is the same? Or Will it tear it down and recreate?

It's my understanding that PhetioGroup doesn't currently re-use indices.  I'm not sure whether RichText rebuilds links on change, but it would be reasonable to expect that it does.  Regarding destroying and re-creating in the same frame, I think that is forbidden by the validator because it checks eagerly.  I couldn't find an issue about making that check "at the end of the frame" instead of eagerly.
It may be easier for us to get it working with PhetioGroup, but if we consider the readability and understandability of the data stream, we should prefer an approach with named (not numbered) links.
For instance, if one version of a sim has 'Hello Friendly Friendly Giant' and the next version has 'Hello Hello!Friendly Friendly Giant', then it would be confusing that link1 has a different meaning in each.

That may be easier, and nice to not expose tandemName in translated strings etc.

If we don't currently have a way to assert that "code" parts of translation strings remain undisturbed, we should probably add that.

It's my understanding that PhetioGroup doesn't currently re-use indices.  I'm not sure whether RichText rebuilds links on change, but it would be reasonable to expect that it does.  Regarding destroying and re-creating in the same frame, I think that is forbidden by the validator because it checks eagerly.  I couldn't find an issue about making that check "at the end of the frame" instead of eagerly.
It may be easier for us to get it working with PhetioGroup, but if we consider the readability and understandability of the data stream, we should prefer an approach with named (not numbered) links.
For instance, if one version of a sim has 'Hello Friendly Friendly Giant' and the next version has 'Hello Hello!Friendly Friendly Giant', then it would be confusing that link1 has a different meaning in each.

That may be easier, and nice to not expose tandemName in translated strings etc.

If we don't currently have a way to assert that "code" parts of translation strings remain undisturbed, we should probably add that.

I think we need to reach out to @jonathanolson to figure out if links are recreated when changing text, even if the same link persists. If the same RichTextLink instances persist, is this an implementation detail that we should rely on?
If we go with supporting tandemName and things get recreated, then each will need a PhetioCapsule to wrap them, or change our validation rules to support regeneration in the same frame. I would hate to use a PhetioCapsule, and would probably prefer just going with PhetioGroup.

For instance, if one version of a sim has . . .

I'm sure that there are other instances in the data stream where changes between versions can make for challenging situations to grok. If one really cares about knowing when a particular link was interacted with, then a unique ID for just that version does not seem like the end of the world to me. That said, the tradeoff is less usability for easier implementation, which in general is not the best philosophy.

I think we need to reach out to @jonathanolson to figure out if links are recreated when changing text, even if the same link persists. If the same RichTextLink instances persist, is this an implementation detail that we should rely on?
If we go with supporting tandemName and things get recreated, then each will need a PhetioCapsule to wrap them, or change our validation rules to support regeneration in the same frame. I would hate to use a PhetioCapsule, and would probably prefer just going with PhetioGroup.

For instance, if one version of a sim has . . .

I'm sure that there are other instances in the data stream where changes between versions can make for challenging situations to grok. If one really cares about knowing when a particular link was interacted with, then a unique ID for just that version does not seem like the end of the world to me. That said, the tradeoff is less usability for easier implementation, which in general is not the best philosophy.

I believe RichTextLinks are recreated on any change.
Also... as a general issue, how does phet-io handle pooled Nodes or PhetioObjects in general? Presumably a RichTextLink would take multiple tandems or hrefs over its lifetime, but my understanding is that this isn't currently possible with phet-io.

I believe RichTextLinks are recreated on any change.
Also... as a general issue, how does phet-io handle pooled Nodes or PhetioObjects in general? Presumably a RichTextLink would take multiple tandems or hrefs over its lifetime, but my understanding is that this isn't currently possible with phet-io.

Each of the proposals seems problematic.  What if we (for now) restrict ourselves to the case where instrumented RichTexts that have no more than 1 link and are never rebuilt?  The fireListener can be myRichText.fireListener.  Simulation cases with RichTexts that have >1 link can be OPT_OUT for now until we have design team discussions.  Also, maybe I am confused about something--I only see one creation of fireListener in RichText.initialize.

Each of the proposals seems problematic.  What if we (for now) restrict ourselves to the case where instrumented RichTexts that have no more than 1 link and are never rebuilt?  The fireListener can be myRichText.fireListener.  Simulation cases with RichTexts that have >1 link can be OPT_OUT for now until we have design team discussions.  Also, maybe I am confused about something--I only see one creation of fireListener in RichText.initialize.

That is not in RichText.initialize, but instead in RichTextLeaf.initialize, which is one of potentially many sub components that are created when a the text is "richified". There are 4 classes in RichText.

That is not in RichText.initialize, but instead in RichTextLeaf.initialize, which is one of potentially many sub components that are created when a the text is "richified". There are 4 classes in RichText.

I'd be curious to see if https://github.com/phetsims/phet-io/issues/1729 is easier that we think. That said, I'm happy to go with @samreid's naive solution for now. I agree that multiple links in the same string aren't very common in phet-io currently.

I'd be curious to see if https://github.com/phetsims/phet-io/issues/1729 is easier that we think. That said, I'm happy to go with @samreid's naive solution for now. I agree that multiple links in the same string aren't very common in phet-io currently.

I'm not sure I understand how this will work...

That is not in RichText.initialize, but instead in RichTextLeaf.initialize

Say RichText A creates RichTextLeaf X with a tandem. Then RichText A changes text and rebuilds (releasing RichTextLeaf X to the pool), and RichTextB pulls it OUT of the pool, trying to give it a new tandem.
Can objects be given a different tandem? I don't see how the "naive solution" works given our current constraints. Can tandems and other properties be reassigned on the same PhetioObject?

I'm not sure I understand how this will work...

That is not in RichText.initialize, but instead in RichTextLeaf.initialize

Say RichText A creates RichTextLeaf X with a tandem. Then RichText A changes text and rebuilds (releasing RichTextLeaf X to the pool), and RichTextB pulls it OUT of the pool, trying to give it a new tandem.
Can objects be given a different tandem? I don't see how the "naive solution" works given our current constraints. Can tandems and other properties be reassigned on the same PhetioObject?

Can objects be given a different tandem

It doesn't really work like this. the Tandem tree is basically a singleton data structure, so if a Tandem has already been created for a phetioID, calling createTandem() to try to make the same phetioID will just grab the same instance, instead of creating two Tandem instances with the same phetioIDs.
Thus I THINK (hesitantly) that something like https://github.com/phetsims/phet-io/issues/1729 may work for the case above with poolable, because you would be recreating the exact same phetioID. But it is untested and would need be thought through more.

Can objects be given a different tandem

It doesn't really work like this. the Tandem tree is basically a singleton data structure, so if a Tandem has already been created for a phetioID, calling createTandem() to try to make the same phetioID will just grab the same instance, instead of creating two Tandem instances with the same phetioIDs.
Thus I THINK (hesitantly) that something like https://github.com/phetsims/phet-io/issues/1729 may work for the case above with poolable, because you would be recreating the exact same phetioID. But it is untested and would need be thought through more.

because you would be recreating the exact same phetioID

I don't see how that's the case. If RichText A has a tandem firstScreen.model.something, and RichText B has tandem secondScreen.view.foo....

because you would be recreating the exact same phetioID

I don't see how that's the case. If RichText A has a tandem firstScreen.model.something, and RichText B has tandem secondScreen.view.foo....

Oh sorry, you are talking about something slightly different from me. I think you are talking about general support for Poolable and PhetioObject. This is untested also, but in theory we can just have dispose do a better job of freeing PhetioObject fields, such that when you initialize it again for the next instance, it can take whatever values it needs to gracefully.
The validation part is all in phetioEngine, where you register PhetioObjects, and presumably you can just unregister ObjectA for TandemX, and then we can work out how to clean it up, and then we can reinitialize ObjectA with TandemY and add that back to phetioEngine, and the engine will be none the wiser.
Again this isn't supported currently, but we should come up with some cases where this is important so that we can prototype.

Oh sorry, you are talking about something slightly different from me. I think you are talking about general support for Poolable and PhetioObject. This is untested also, but in theory we can just have dispose do a better job of freeing PhetioObject fields, such that when you initialize it again for the next instance, it can take whatever values it needs to gracefully.
The validation part is all in phetioEngine, where you register PhetioObjects, and presumably you can just unregister ObjectA for TandemX, and then we can work out how to clean it up, and then we can reinitialize ObjectA with TandemY and add that back to phetioEngine, and the engine will be none the wiser.
Again this isn't supported currently, but we should come up with some cases where this is important so that we can prototype.

Here are some options:

DEFER: Maybe what would help this issue the most is if we could tie it to a deliverable.  If we can decide that, for the moment, we don't need a clickstream on the links in the about dialog, then things can remain as OPT_OUT.  Then when we have a PhET-iO sim that requires a clickstream for RichText links, we can address it then with context and support from designers.
HANDLE SIMPLE: What if we (for now) restrict ourselves to the case where instrumented RichTexts that have no more than 1 link and are never rebuilt?  The fireListener can be myRichText.fireListener.  Simulation cases with RichTexts that have >1 link can be OPT_OUT for now until we have design team discussions.  Also, maybe I am confused about something--I only see one creation of fireListener in RichText.initialize.
PHETIO_GROUP: Use PhetioGroup to increment indices of listeners each time they are recreated.

Am I missing other workable options?  Which do we prefer at the moment?

Here are some options:

DEFER: Maybe what would help this issue the most is if we could tie it to a deliverable.  If we can decide that, for the moment, we don't need a clickstream on the links in the about dialog, then things can remain as OPT_OUT.  Then when we have a PhET-iO sim that requires a clickstream for RichText links, we can address it then with context and support from designers.
HANDLE SIMPLE: What if we (for now) restrict ourselves to the case where instrumented RichTexts that have no more than 1 link and are never rebuilt?  The fireListener can be myRichText.fireListener.  Simulation cases with RichTexts that have >1 link can be OPT_OUT for now until we have design team discussions.  Also, maybe I am confused about something--I only see one creation of fireListener in RichText.initialize.
PHETIO_GROUP: Use PhetioGroup to increment indices of listeners each time they are recreated.

Am I missing other workable options?  Which do we prefer at the moment?

Deferring seems natural until we hit a case where this would be used. Perhaps we'll have support for changing tandems on objects by then?

Deferring seems natural until we hit a case where this would be used. Perhaps we'll have support for changing tandems on objects by then?

That sounds reasonable to me, unassigning.

That sounds reasonable to me, unassigning.

I like "HANDLE SIMPLE".  It is easy and I don't see any usages of RichText Links besides UnderDevelopmentPlane and the about dialog link texts. To me this is the best solution, and when we have a PhET-iO case that needs more, we can reevaluate (which is what @jonathanolson seems to be calling for). I feel like it is unlikely that we will encounter multiple links in a PhET-iO context any time soon, and with this patch below, we will get events like:
{
          "index": 309,
          "time":<PHONE_NUMBER>569,
          "type": "USER",
          "phetioID": "gravityAndOrbits.general.view.navigationBar.phetButton.phetMenu.aboutDialogCapsule.aboutDialog.termsPrivacyAndLicensingLinkText.fireListener.firedEmitter",
          "name": "emitted",
          "componentType": "EmitterIO<NullableIO<SceneryEventIO>>",
          "data": {
            "event": {
              "type": "up",
              "domEventType": "pointerup",
              "point": {
                "x": 348,
                "y": 606
              }
            }
          }

Index: js/nodes/RichText.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/nodes/RichText.js b/js/nodes/RichText.js
--- a/js/nodes/RichText.js	(revision 9a0fd3e4db6a4ce0fe0111f8b5b40cdeb6955dcc)
+++ b/js/nodes/RichText.js	(date<PHONE_NUMBER>023)
@@ -60,10 +60,10 @@
 import StringProperty from '../../../axon/js/StringProperty.js';
 import TinyForwardingProperty from '../../../axon/js/TinyForwardingProperty.js';
 import Matrix3 from '../../../dot/js/Matrix3.js';
-import Poolable from '../../../phet-core/js/Poolable.js';
 import extendDefined from '../../../phet-core/js/extendDefined.js';
 import merge from '../../../phet-core/js/merge.js';
 import openPopup from '../../../phet-core/js/openPopup.js';
+import Poolable from '../../../phet-core/js/Poolable.js';
 import Tandem from '../../../tandem/js/Tandem.js';
 import IOType from '../../../tandem/js/types/IOType.js';
 import FireListener from '../listeners/FireListener.js';
@@ -229,6 +229,10 @@
       phetioType: RichText.RichTextIO
     }, options );

+    // @private - For creating sub tandems in rebuildRichText() which occur before PhetioObject.initializePhetioObject(
+    // is called on this RichText. This cannot be named `tandem` or else instrumentation in Node doesn't occur correctly.
+    this.tempRichTextTandem = options.tandem;
+
     // @private {Node} - Normal layout container of lines (separate, so we can clear it easily)
     this.lineContainer = new Node( {} );
     this.addChild( this.lineContainer );
@@ -409,7 +413,7 @@
           }
         }

-        const linkRootNode = RichTextLink.createFromPool( linkElement.innerContent, href );
+        const linkRootNode = RichTextLink.createFromPool( linkElement.innerContent, href, this.tempTandem );
         this.lineContainer.addChild( linkRootNode );

         // Detach the node from its location, adjust its transform, and reattach under the link. This should keep each
@@ -450,6 +454,7 @@
    */
   dispose() {
     this.freeChildrenToPool();
+    this.tempRichTextTandem = null;

     super.dispose();

@@ -1832,8 +1837,9 @@
    *
    * @param {string} innerContent
    * @param {function|string} href
+   * @param {Tandem} tandem - keep this as an arg so that we don't instrument this Node (it is just an intermediary)
    */
-  constructor( innerContent, href ) {
+  constructor( innerContent, href, tandem ) {
     super( {
       cursor: 'pointer',
       tagName: 'a'
@@ -1845,7 +1851,7 @@
     // @private {function|null}
     this.accessibleInputListener = null;

-    this.initialize( innerContent, href );
+    this.initialize( innerContent, href, tandem );
   }

   /**
@@ -1854,9 +1860,10 @@
    *
    * @param {string} innerContent
    * @param {function|string} href
+   * @param {Tandem} tandem - keep this as an arg so that we don't instrument this Node (it is just an intermediary)
    * @returns {RichTextLink} - Self reference
    */
-  initialize( innerContent, href ) {
+  initialize( innerContent, href, tandem ) {
     // pdom - open the link in the new tab when activated with a keyboard.
     // also see https://github.com/phetsims/joist/issues/430
     this.innerContent = innerContent;
@@ -1865,7 +1872,7 @@
     if ( typeof href === 'function' ) {
       this.fireListener = new FireListener( {
         fire: href,
-        tandem: Tandem.OPT_OUT
+        tandem: tandem.createTandem( 'fireListener' )
       } );
       this.addInputListener( this.fireListener );
       this.setAccessibleAttribute( 'href', '#' ); // Required so that the click listener will get called.
@@ -1891,7 +1898,7 @@
           self._linkEventsHandled && event.handle();
           openPopup( href ); // open in a new window/tab
         },
-        tandem: Tandem.OPT_OUT
+        tandem: tandem.createTandem( 'fireListener' )
       } );
       this.addInputListener( this.fireListener );
       this.setAccessibleAttribute( 'href', href );
@@ -1914,6 +1921,7 @@
     }

     this.removeInputListener( this.fireListener );
+    this.fireListener.dispose();
     this.fireListener = null;
     if ( this.accessibleInputListener ) {
       this.removeInputListener( this.accessibleInputListener );

Note the weirdness about having access to a tandem before mutate is called (and even while mutate occurs but it hasn't yet become an initialized PhetioObject yet like in setText). That is a bit of a bummer, but if that could be a bit better, then I think this is the way to go.
What do you think SR and JO?

I like "HANDLE SIMPLE".  It is easy and I don't see any usages of RichText Links besides UnderDevelopmentPlane and the about dialog link texts. To me this is the best solution, and when we have a PhET-iO case that needs more, we can reevaluate (which is what @jonathanolson seems to be calling for). I feel like it is unlikely that we will encounter multiple links in a PhET-iO context any time soon, and with this patch below, we will get events like:
{
          "index": 309,
          "time":<PHONE_NUMBER>569,
          "type": "USER",
          "phetioID": "gravityAndOrbits.general.view.navigationBar.phetButton.phetMenu.aboutDialogCapsule.aboutDialog.termsPrivacyAndLicensingLinkText.fireListener.firedEmitter",
          "name": "emitted",
          "componentType": "EmitterIO<NullableIO<SceneryEventIO>>",
          "data": {
            "event": {
              "type": "up",
              "domEventType": "pointerup",
              "point": {
                "x": 348,
                "y": 606
              }
            }
          }

Index: js/nodes/RichText.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/nodes/RichText.js b/js/nodes/RichText.js
--- a/js/nodes/RichText.js	(revision 9a0fd3e4db6a4ce0fe0111f8b5b40cdeb6955dcc)
+++ b/js/nodes/RichText.js	(date<PHONE_NUMBER>023)
@@ -60,10 +60,10 @@
 import StringProperty from '../../../axon/js/StringProperty.js';
 import TinyForwardingProperty from '../../../axon/js/TinyForwardingProperty.js';
 import Matrix3 from '../../../dot/js/Matrix3.js';
-import Poolable from '../../../phet-core/js/Poolable.js';
 import extendDefined from '../../../phet-core/js/extendDefined.js';
 import merge from '../../../phet-core/js/merge.js';
 import openPopup from '../../../phet-core/js/openPopup.js';
+import Poolable from '../../../phet-core/js/Poolable.js';
 import Tandem from '../../../tandem/js/Tandem.js';
 import IOType from '../../../tandem/js/types/IOType.js';
 import FireListener from '../listeners/FireListener.js';
@@ -229,6 +229,10 @@
       phetioType: RichText.RichTextIO
     }, options );

+    // @private - For creating sub tandems in rebuildRichText() which occur before PhetioObject.initializePhetioObject(
+    // is called on this RichText. This cannot be named `tandem` or else instrumentation in Node doesn't occur correctly.
+    this.tempRichTextTandem = options.tandem;
+
     // @private {Node} - Normal layout container of lines (separate, so we can clear it easily)
     this.lineContainer = new Node( {} );
     this.addChild( this.lineContainer );
@@ -409,7 +413,7 @@
           }
         }

-        const linkRootNode = RichTextLink.createFromPool( linkElement.innerContent, href );
+        const linkRootNode = RichTextLink.createFromPool( linkElement.innerContent, href, this.tempTandem );
         this.lineContainer.addChild( linkRootNode );

         // Detach the node from its location, adjust its transform, and reattach under the link. This should keep each
@@ -450,6 +454,7 @@
    */
   dispose() {
     this.freeChildrenToPool();
+    this.tempRichTextTandem = null;

     super.dispose();

@@ -1832,8 +1837,9 @@
    *
    * @param {string} innerContent
    * @param {function|string} href
+   * @param {Tandem} tandem - keep this as an arg so that we don't instrument this Node (it is just an intermediary)
    */
-  constructor( innerContent, href ) {
+  constructor( innerContent, href, tandem ) {
     super( {
       cursor: 'pointer',
       tagName: 'a'
@@ -1845,7 +1851,7 @@
     // @private {function|null}
     this.accessibleInputListener = null;

-    this.initialize( innerContent, href );
+    this.initialize( innerContent, href, tandem );
   }

   /**
@@ -1854,9 +1860,10 @@
    *
    * @param {string} innerContent
    * @param {function|string} href
+   * @param {Tandem} tandem - keep this as an arg so that we don't instrument this Node (it is just an intermediary)
    * @returns {RichTextLink} - Self reference
    */
-  initialize( innerContent, href ) {
+  initialize( innerContent, href, tandem ) {
     // pdom - open the link in the new tab when activated with a keyboard.
     // also see https://github.com/phetsims/joist/issues/430
     this.innerContent = innerContent;
@@ -1865,7 +1872,7 @@
     if ( typeof href === 'function' ) {
       this.fireListener = new FireListener( {
         fire: href,
-        tandem: Tandem.OPT_OUT
+        tandem: tandem.createTandem( 'fireListener' )
       } );
       this.addInputListener( this.fireListener );
       this.setAccessibleAttribute( 'href', '#' ); // Required so that the click listener will get called.
@@ -1891,7 +1898,7 @@
           self._linkEventsHandled && event.handle();
           openPopup( href ); // open in a new window/tab
         },
-        tandem: Tandem.OPT_OUT
+        tandem: tandem.createTandem( 'fireListener' )
       } );
       this.addInputListener( this.fireListener );
       this.setAccessibleAttribute( 'href', href );
@@ -1914,6 +1921,7 @@
     }

     this.removeInputListener( this.fireListener );
+    this.fireListener.dispose();
     this.fireListener = null;
     if ( this.accessibleInputListener ) {
       this.removeInputListener( this.accessibleInputListener );

Note the weirdness about having access to a tandem before mutate is called (and even while mutate occurs but it hasn't yet become an initialized PhetioObject yet like in setText). That is a bit of a bummer, but if that could be a bit better, then I think this is the way to go.
What do you think SR and JO?

First of all, the fireListener.dispose() seems like it should be added (as it's overwritten anyway every time).
That... seems like it works, although it will fail out if you provide a tandem and 2+ links (what happens if you provide no tandem and 2+ links?)
I'd like to hear @samreid's thoughts.

First of all, the fireListener.dispose() seems like it should be added (as it's overwritten anyway every time).
That... seems like it works, although it will fail out if you provide a tandem and 2+ links (what happens if you provide no tandem and 2+ links?)
I'd like to hear @samreid's thoughts.

The patch has a mismatched variable name, tempTandem should be renamed to tempRichTextTandem.  After I tried that, I added 2 URLs to one RichText in AboutDialog, and got this error:

Assertion failed: phetioID already registered: gravityAndOrbits.general.view.navigationBar.phetButton.phetMenu.aboutDialogCapsule.aboutDialog.termsPrivacyAndLicensingLinkText.fireListener.pressAction

The proposal from @zepumph therefore seems like a great solution for now, any RichText with 2+ links will loudly error out.

First of all, the fireListener.dispose() seems like it should be added (as it's overwritten anyway every time).

The patch does have a fireListener.dispose() so I'm confused about this comment.  Back to @zepumph

The patch has a mismatched variable name, tempTandem should be renamed to tempRichTextTandem.  After I tried that, I added 2 URLs to one RichText in AboutDialog, and got this error:

Assertion failed: phetioID already registered: gravityAndOrbits.general.view.navigationBar.phetButton.phetMenu.aboutDialogCapsule.aboutDialog.termsPrivacyAndLicensingLinkText.fireListener.pressAction

The proposal from @zepumph therefore seems like a great solution for now, any RichText with 2+ links will loudly error out.

First of all, the fireListener.dispose() seems like it should be added (as it's overwritten anyway every time).

The patch does have a fireListener.dispose() so I'm confused about this comment.  Back to @zepumph

any RichText with 2+ links will loudly error out

Any RichText with 2+ links AND tandems hopefully?

The patch does have a fireListener.dispose() so I'm confused about this comment.

Agreed, that should have existed before the patch, and was an omission on my part!

Any RichText with 2+ links AND tandems hopefully?

Yes, that's my understanding.

I'm happy to implement and test on my next iO day.

Added the missing fireListener dispose in the commit above.

Ok great. Thanks! Closing


[CT-1025] Should adapters be loaded earlier?

While looking at #5620, the fact that adapters are loaded only as part of ConfiguredTask came up. This means that anywhere in the codebase that uses get_adapter_package_names will not have access to the packages that adapters use.
This ticket tracks the effort around finding out if this behavior should be modified or remain as is.

Specific tasks where we've seen this come up: init + dbt --version (not really a task)
Instead of truly loading these adapters, dbt just looks in the Python-installed file system for:
https://github.com/dbt-labs/dbt-core/blob/4d6208be6464a48a078dd8000ae6c29d8d7a42ce/core/dbt/version.py#L221-L235
So all adapter factory methods, while available, aren't really usable.


New Project on issue #29

🛠 Issue (Number) --#29

Closes #
Description
Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change.
Type of change

[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

Checklist:

[ ] I have made this from my own
[x] I have taken help from some online resourses
[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[x] The title of my pull request is a short description of the requested changes.

ATTACH SCREEN-SHOTS / DEPLOYMENT LINK

If applicable, add screenshots/deploying link.

Kindly merge the request

Kindly review the request at earliest


Update Whitehall change note label

This label was changed in
https://github.com/alphagov/whitehall/pull/5604
Trello

No longer needed as content not changing


add task github issue template

I want to create new issue to assign myself a task

Could you also add some info to help people understand when they should use a feature request vs. a task? Is a task just for non-code things?

Feature request is like a Story to me (agreed?) Task represents work that needs to be done. (Sub-)task would be underneath a Feature request (Story) which in our case (R&D) might just stand in the way. I suggest to bend the rules then.
Task can include non-code and most likely will i.e. study/investigate X , do a PoC of Y (pseudo-code?) , get more info about something (+writing a documentation is a task)
cc Alexey Kuznetsov (TODO: mention)

OK to merge after addressing these comments with no further review.

How? Merge commit or rebase?

So the hierarchy is going to be: Milestones -> Features -> Tasks. Three levels make sense. I think we need to rename "Features" to something else then. Like "Stories". But then all previously created features might not be happy about this :)
Anyway, let's give it a go.

So the hierarchy is going to be: Milestones -> Features -> Tasks.

I'm advocating about purpose, not hierarchy. It's R&D. It is not a process of writing down to spec software with well defined specs, we generally "don't know what we are doing" :)
Features are just like that, user stories , i.e. features to be added to existing software. @jkff whoever came up with this in first place, please explain rational behind if I'm not correct
I suggest move discussion to Slack to get team's opinion...

I think we need to rename "Features" to something else then.

No, we don't. :-P


Fix all lint errors.

I was just testing CircleCI integration with this, which looks like it's working. This was merged with https://github.com/google/incremental-dom/pull/470


[GSI_LOGGER]: The given origin is not allowed for the given client ID.

Hi, im trying to add google login to my react app, and i receive this error every time:

Failed to load resource: the server responded with a status of 400 ()
[GSI_LOGGER]: The given origin is not allowed for the given client ID.

Im running the site on django, port 8000, frontend is writed on react, using webpack.
I setuped all like in the tutorial, added both of the urls, and it still not working.

Can i receive some help please?

Hello,
use the same port as the react app is using

Hello,
use the same port as the react app is using

Im not using react app to launch the site, django is running, on the port 8000, i dont have any other running ports.

The package is meant to be used inside react apps,
if you need the native js implementation you can check google docs, or copy-paste from the package source code what works for you

Okay, thanks for the answer


103 line of MPLS - wrong comment

Pull request #635 submitted to address this.

It would be more appropriate to merge all pull requests into devel branch because master serves as a clean snapshot of latest released version.

Makes sense, shall I revert the change and submit new pull request to devel?

@dhenza yes, please.


[BUG] Unable To Extract Page Data For URL which is Downloading Content Automatically

Environment

chrome-aws-lambda Version: 705776
puppeteer / puppeteer-core Version: 2.0.*
OS: AWS Lambda
Node.js Version: 12.x
Lambda / GCF Runtime: 2538

Expected Behavior
page.goto should work
Current Behavior
ERROR	Invoke Error
{
"errorType": "Error",
"errorMessage": "[object Object]",
"stack": [
"Error: [object Object]",
"    at _homogeneousError (/var/runtime/CallbackContext.js:13:12)",
"    at postError (/var/runtime/CallbackContext.js:30:54)",
"    at done (/var/runtime/CallbackContext.js:57:7)",
"    at fail (/var/runtime/CallbackContext.js:69:7)",
"    at Object.fail (/var/runtime/CallbackContext.js:105:16)",
"    at Runtime.exports.handler (/var/task/index.js:126:20)"
]
}
Steps to Reproduce
https://www.malwaredomainlist.com/hostslist/yesterday_urls.php

Reference: https://github.com/alixaxel/chrome-aws-lambda/issues/123

Referenced code is no longer available (and is not hosted on an insecure site with an expired web certificate).


ci: Allow PRs from external forks to run (specific) workflows

Why it needs to get done
Context
At the moment, all workflows in our CI require credentials. People outside of the Kubeflow GH group, sending PRs from a fork, won't be able to run any of the GH workflows. This is by design since we want to avoid access to our repos' credentials/secrets.
What we typically do is someone from our team will duplicate the PR to be able to run the CI. This goes both for parts of our CI that actually require credentials (like publish jobs and lib-check) but also for the rest of them (lint, unit and integration tests)
Goals

Ensure contributions from users outside the Kubeflow GH group can be merged in a standardized/smooth way while at the same, PRs run all must-pass workflows.
Avoid the hassle of having to duplicate PRs.

What needs to get done

Remove the CHARMCRAFT_CREDENTIALS.required: true from our CI in order to enable workflows to run
Add a condition to lib-check job in order to run only when credentials secret is available. This way, on PRs from forks, the job will be Skipped (instead of Failed) while it will run on internal PRs. We 're OK with omitting this check.
Refactor publish job in order to ensure that the job can build the charm (without publishing it). This way, on PRs from forks, we 'll have the job only building the charm on PRs from forks. Note that we already build the charms during integration tests but in the past (and I 'm quoting @ca-scribner on this), we 've seen some discrepancies between the output from the publish jobs and charmcraft pack in our tests.

When is the task considered done

PRs from forks run all CI apart from lib-check job and publish part of publish job

link for the comment above about how we ship different images than we test: https://github.com/canonical/bundle-kubeflow/issues/766


Remove unnessesary test

I removed test ClusteredAgentManagerServiceTest.testOther because the test is meaningless and affects other test.

LGTM