Various fixes for ordered_yaml #2
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Whatever the value to produce is "true" or "false", only "true" is outputted. Output the appropriate value instead.
If the structure to convert contains "true" as a string, it would be converted to a boolean. The same applies for a string with an integer or a float. This seems a bad idea. For example, when specifying the "true" command, we get the "true" boolean. Or when we put "0100" as a PIN code, we get 64 instead. The problem may be similar in ordered_json.
|
Thanks, unfortunately we don't accept pull requests in github. Our canonical repo is hosted at https://gerrit.wikimedia.org Please submit a patchset there :) |
|
Sorry, too much hassle for me to do that. Feel free to submit the first commit if you think it's correct. |
|
I've filed an issue upstream in our issue tracker assigned to the author of that script for him to decide whether to incorporate it |
|
@vincentbernat, thank you -- I really appreciate it. I merged your changes in https://gerrit.wikimedia.org/r/#/c/257075/. |
|
@atdt, I see this has been reverted. I didn't look at the code carefully enough and didn't notice that |
wmfgerrit
pushed a commit
that referenced
this pull request
Feb 25, 2016
Change-Id: Icdbd0886eaceb496341e81ada3fd11b663adff05
wmfgerrit
pushed a commit
that referenced
this pull request
Jul 12, 2016
This is similar in purpose to the simpler attempt earlier in https://gerrit.wikimedia.org/r/#/c/294464 which was reverted because vcl_backend_response can't set req.http.*. This variant communicates through beresp->resp, and prevents applayer interference in that mechanism. Change-Id: Id205b1ba79a44a8d21ab1b3fa61fd337ab59f25d
wmfgerrit
pushed a commit
that referenced
this pull request
Jul 6, 2017
Fix one more instance of this variable Change-Id: I69858afa8b451137767f971e88fcfbdeb6e646af
wmfgerrit
pushed a commit
that referenced
this pull request
May 28, 2019
This is the 3º patch attempt. First 2 were reverted because: * attempt #1 produced some broken CloudVPS VMs * attempt #2 was merged with not enough review Callers of the sudo module can choose which sudo flavor they want: * sudo (normal sudo, the default) * sudo-ldap (a LDAP/nscd/nslcd based sudo) The sudo-ldap flavor is usually used in CloudVPS, but if using sssd rather than nscd/nslcd we need sudo instead of sudo-ldap. This patch involves decoupling the sudo class into another subclass called sudo::sudoldap, which contains the specific bits related to that version of sudo. The desired outcome is that a CloudVPS VM can choose which LDAP client stack to use between 2: * classic, using nscd/nslcd, and sudo-ldap * sssd, using the sssd daemon and sudo (no sudo-ldap) Both flavors are incompatible. A new global hiera key is introduced: 'sudo_flavor', which defaults to 'sudoldap' in CloudVPS and to 'sudo' in prod. Eventually, this hiera key will be dropped, since we aim to a near future in which we run sssd in CloudVPS and we can just use 'sudo' everywhere. Bug: T221225 Change-Id: I5481218bd044fa6c46d8a66cd60e9d58b339e200 Signed-off-by: Arturo Borrero Gonzalez <[email protected]>
wmfgerrit
pushed a commit
that referenced
this pull request
Sep 13, 2019
* Take #1 was a change to the mediawiki config which stopped directing the logs to the mediawiki type, but the logs never made it to logstash and were probably getting filtered out by puppet code. * This version relies on no change to the mediawiki config. I'll revert my other patch. Bug: T232042 Depends-On: Ie164a66bcf3d0fa05bd7f9258e4389150fdf5a2b Change-Id: Id6c7050aee22fa80512bb8f375055d442e9d1d7d
wmfgerrit
pushed a commit
that referenced
this pull request
Nov 26, 2020
Ship the regenerated certificated Bug: T268747 Change-Id: I37daf7b1f7d0cb5bdfb83b609a230815ac88316c
wmfgerrit
pushed a commit
that referenced
this pull request
Feb 25, 2021
There was an issue with the salt previously, but that's fixed now. Bug: T275559 Change-Id: Iadf494d56b717c78be04149d76ffa93a0f37a959
wmfgerrit
pushed a commit
that referenced
this pull request
Dec 14, 2021
This reverts commit 0b64e30. Reason for revert: underlying issue fixed. Bug: T263277 Change-Id: I030202e9c37fcc25ba4900b1aa87cd1948461d71
wmfgerrit
pushed a commit
that referenced
this pull request
Oct 10, 2022
Partitioning the ATS cache effectively wipes it so we need to proceed one server per cluster and DC at a time to avoid overwhelming the applayer. Affected servers: * cp1087 * cp1088 * cp2039 * cp2040 * cp3062 * cp3063 * cp4033 * cp4035 * cp5013 * cp5015 * cp6006 * cp6014 ATS needs to be restarted to apply the new cache partition schema Change-Id: Ia23d4cab2061e32a58df7a8a6ce5f16fd0be484b
wmfgerrit
pushed a commit
that referenced
this pull request
Oct 17, 2022
Change-Id: I1360746d4b0d0224d305edf2bf1117344e90515b
wmfgerrit
pushed a commit
that referenced
this pull request
Jan 11, 2023
Change-Id: Ib2295569f7be346415b9824a9809c9970d503ffd
wmfgerrit
pushed a commit
that referenced
this pull request
Feb 9, 2023
[VARIOUS NOTES ON CONFIG VALUES WE CHOSE] Link #1 https://logging.apache.org/log4j/2.x/manual/layouts.html: - If complete="false", the appender does not write the JSON open array character "[" at the start of the document, "]" and the end, nor comma "," between records. ^ We don't care about the whole file being readable json (rather than each newline-delimited message) so we have set complete="false" Link #2 https://www.elastic.co/guide/en/elasticsearch/reference/7.10/logging.html#json-logging ^ This is where we got ESJsonLayout from Hosts: relforge1003.eqiad.wmnet,cloudelastic1006.wikimedia.org Hosts: elastic2070.codfw.wmnet,elastic1070.eqiad.wmnet Bug: T324335 Change-Id: I7385107f8922d566bf8fbf7cb61dd3211d662822
wmfgerrit
pushed a commit
that referenced
this pull request
Mar 30, 2023
Take #2 now that we have ipv6 available on graphite/statsd Bug: T239862 Change-Id: I98e0cf36e557142776809871058bf62b35e8f3d3
wmfgerrit
pushed a commit
that referenced
this pull request
May 30, 2023
Take #2, now with the correct flag Bug: T337689 Change-Id: If5dd7d377e9367c0400bc26caf203b39bd9e4c8d
wmfgerrit
pushed a commit
that referenced
this pull request
Jun 13, 2023
The Zuul scheduler has two connections to Gerrit:
- a source one which is the stream events interface to receive events.
- a reporting one to do reviews of changes
Our configuration has both kind under the same name `gerrit` and the
connection instances are stored in a dict which is used to retrieve the
instance:
{
"gerrit": <GerritConnection #1>
}
When the source fails and reestablishes the connection and at the same
time a reporter tries to emit a report, they both receive the same
instance (albeit in different threads) and enter a race condition which
causes a variable to be replaced while the ssh connection stays
established. As a result we have an extra useless connection idling
which goes against the 4 ssh session per user imposed by our Gerrit and
leads to zuul merger not being able to establish a connection.
To workaround the lack of lock on the GerritConnection variable, this
creates a new connection `gerrit-reporter` on the Zuul server.
Internally it would thus be stored under a different key:
{
"gerrit": <GerritConnection #1>,
"gerrit-reporter": <GerritConnection #2>
}
The source and reporter would thus no more share the same instance and
that should workaround the race condition.
This should not affect any operation until we change the Zuul layout in
integration/config to start reporting using this new connection:
pipelines:
- name: test
success:
- gerrit:
+ gerrit-reporter:
verified: 2
tag: autogenerated:ci-test
It should be a noop for the zuul-merge.conf.
Bug: T309376
Hosts: contint2001.wikimedia.org
Hosts: contint1002.wikimedia.org
Change-Id: I0e7369b8f6c533498c2abad75ab7da439a94572e
wmfgerrit
pushed a commit
that referenced
this pull request
Jul 3, 2023
We are switching kubestagemaster to a VIP, since we will be having multiple kubestagemasters. Bug: T329827 Change-Id: I4b8d8725a3ac4afc1ed81afe929e54b4478d16c3
wmfgerrit
pushed a commit
that referenced
this pull request
Jul 20, 2023
add prometheus-https instance to send port 443 to the same backends used for prometheus on port 80 (attempt #2) Bug: T301944 Bug: T326657 Change-Id: Ib77cf3872ed564a982c5d668d448ade7beedf633
wmfgerrit
pushed a commit
that referenced
this pull request
Nov 27, 2023
1fa6e08 missed adding a explicit relationship between Augeas[$iface_add_up] and interface::manual Change-Id: Ib68be5353bedc6d047ec1e8ac008544d433d451e
wmfgerrit
pushed a commit
that referenced
this pull request
Dec 11, 2023
This time, the PyBal healthchecks should pass.
```
brouberol@lvs1019:~$ for i in $(seq 1 8); do \
echo dse-k8s-worker100${i}.eqiad.wmnet && \
nc -z -v -w5 $(dig +short dse-k8s-worker100${i}.eqiad.wmnet) 30443; \
done
dse-k8s-worker1001.eqiad.wmnet
Connection to 10.64.0.38 30443 port [tcp/*] succeeded!
dse-k8s-worker1002.eqiad.wmnet
Connection to 10.64.16.47 30443 port [tcp/*] succeeded!
dse-k8s-worker1003.eqiad.wmnet
Connection to 10.64.32.178 30443 port [tcp/*] succeeded!
dse-k8s-worker1004.eqiad.wmnet
Connection to 10.64.48.52 30443 port [tcp/*] succeeded!
dse-k8s-worker1005.eqiad.wmnet
Connection to 10.64.130.6 30443 port [tcp/*] succeeded!
dse-k8s-worker1006.eqiad.wmnet
Connection to 10.64.132.8 30443 port [tcp/*] succeeded!
dse-k8s-worker1007.eqiad.wmnet
Connection to 10.64.134.5 30443 port [tcp/*] succeeded!
dse-k8s-worker1008.eqiad.wmnet
Connection to 10.64.136.7 30443 port [tcp/*] succeeded!
```
Bug: T352639
Change-Id: Ie9e8a7986898df7bacbf464f486cf6b0a44318e5
wmfgerrit
pushed a commit
that referenced
this pull request
Mar 25, 2024
I goofed I02d456822 and this is the proper change. I've verified that passing usedonly= does the right thing. Bug: T359633 Change-Id: Ic70ea1b3407f0815d0c29751aa30dc9a69700ad8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey!
I did steal your ordered_yaml implementation but I did notice two bugs:
I did two commits as I am pretty sure the first one corrects a bug. However, I suppose that the behaviour corrected by the second one was a wanted behaviour (reflected also in the example). Feel free to discard it. The commit message contains some rationale on why I think this is a bad idea to do this kind of implicit conversions.