nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: canonical/operator
Pull request #936 (2023-05-24 11:57:20)

Remove "dying" units from planned units
This PR addresses an issue that happens during a scale-down event, where `planned_units` includes units with the status marked as `dying`.
This becomes visible when say the `_storage_detaching` hook errors based on certain preconditions preventing a unit from immediately disappearing. 
This PR essentially filters out those dying units to have an accurate count of live units.


Patch:
diff --git a/ops/model.py b/ops/model.py
index 465fbab..24baca9 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -2931,16 +2931,21 @@ class _ModelBackend:
     def planned_units(self) -> int:
         """Count of "planned" units that will run this application.
 
-        Includes the current unit in the count.
+        This will include the current unit, any units that are alive, units that are in the process
+        of being started, but will not include units that are being shut down.
 
         """
         # The goal-state tool will return the information that we need. Goal state as a general
         # concept is being deprecated, however, in favor of approaches such as the one that we use
         # here.
         app_state = self._run('goal-state', return_output=True, use_json=True)
-        app_state = typing.cast(Dict[str, List[str]], app_state)
+        app_state = typing.cast(Dict[str, Dict[str, Any]], app_state)
+
         # Planned units can be zero. We don't need to do error checking here.
-        return len(app_state.get('units', []))
+        # But we need to filter out dying units as they may be reported before being deleted
+        units = app_state.get('units', {})
+        num_alive = sum(1 for unit in units.values() if unit['status'] != 'dying')
+        return num_alive
 
     def update_relation_data(self, relation_id: int, _entity: 'UnitOrApplication',
                              key: str, value: str):


Test patch:
diff --git a/test/test_model.py b/test/test_model.py
index 92a78bd..7b7344a 100755
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -2712,6 +2712,35 @@ exit 2
             ['relation-list', '-r', '6', '--app', '--format=json'],
         ])
 
+    def test_planned_units(self):
+        # no units
+        fake_script(self, 'goal-state', """
+echo '{"units":{}, "relations":{}}'
+""")
+        self.assertEqual(self.backend.planned_units(), 0)
+
+        # only active units
+        fake_script(self, 'goal-state', """
+echo '{
+    "units":{
+        "app/0": {"status":"active","since":"2023-05-23 17:05:05Z"},
+        "app/1": {"status":"active","since":"2023-05-23 17:57:05Z"}
+    },
+    "relations": {}
+}'""")
+        self.assertEqual(self.backend.planned_units(), 2)
+
+        # active and dying units
+        fake_script(self, 'goal-state', """
+echo '{
+    "units":{
+        "app/0": {"status":"active","since":"2023-05-23 17:05:05Z"},
+        "app/1": {"status":"dying","since":"2023-05-23 17:57:05Z"}
+    },
+    "relations": {}
+}'""")
+        self.assertEqual(self.backend.planned_units(), 1)
+
 
 class TestLazyMapping(unittest.TestCase):
Repository: canonical/operator
Pull request #965 (2023-07-05 14:42:23)

Add `has_open_port_k8s` to `JujuVersion`
open-port is not supported before Juju 3.0.3 on Kubernetes

Context: https://chat.charmhub.io/charmhub/pl/91t6ip37ybncuqe11eudfx38xw


Patch:
diff --git a/ops/jujuversion.py b/ops/jujuversion.py
index c5d075c..e026273 100755
--- a/ops/jujuversion.py
+++ b/ops/jujuversion.py
@@ -123,3 +123,9 @@ class JujuVersion:
         # * In 3.0.3, a bug with observer labels was fixed (juju/juju#14916)
         # TODO(benhoyt): update to 3.0.3+ once shipped (for juju/juju#14916 fix)
         return (self.major, self.minor, self.patch) >= (3, 0, 2)
+
+    @property
+    def supports_open_port_on_k8s(self) -> bool:
+        """Report whether this Juju version supports open-port on Kubernetes."""
+        # Support added: https://bugs.launchpad.net/juju/+bug/1920960
+        return (self.major, self.minor, self.patch) >= (3, 0, 3)


Test patch:
diff --git a/test/test_jujuversion.py b/test/test_jujuversion.py
index 0d180a6..6851179 100755
--- a/test/test_jujuversion.py
+++ b/test/test_jujuversion.py
@@ -70,9 +70,16 @@ class TestJujuVersion(unittest.TestCase):
 
     def test_has_secrets(self):
         self.assertTrue(ops.JujuVersion('3.0.2').has_secrets)
+        self.assertTrue(ops.JujuVersion('3.1.0').has_secrets)
         self.assertFalse(ops.JujuVersion('3.0.1').has_secrets)
         self.assertFalse(ops.JujuVersion('2.9.30').has_secrets)
 
+    def test_supports_open_port_on_k8s(self):
+        self.assertTrue(ops.JujuVersion('3.0.3').supports_open_port_on_k8s)
+        self.assertTrue(ops.JujuVersion('3.3.0').supports_open_port_on_k8s)
+        self.assertFalse(ops.JujuVersion('3.0.2').supports_open_port_on_k8s)
+        self.assertFalse(ops.JujuVersion('2.9.30').supports_open_port_on_k8s)
+
     def test_parsing_errors(self):
         invalid_versions = [
             "xyz",
Repository: canonical/operator
Pull request #998 (2023-08-28 03:32:31)

Raise error in wait_output if stdout is already being consumed
When the `stdout` argument is provided to `ops.Container.exec` for capturing the stdout stream, the `ExecProcess` will have a `stdout` attribute of `None`. To prevent potential null safety issues, an error is now raised in such cases within `wait_output`.


Patch:
diff --git a/ops/pebble.py b/ops/pebble.py
index f8ce123..8d107cb 100644
--- a/ops/pebble.py
+++ b/ops/pebble.py
@@ -1331,6 +1331,12 @@ class ExecProcess(Generic[AnyStr]):
             ChangeError: if there was an error starting or running the process.
             ExecError: if the process exits with a non-zero exit code.
         """
+        if self.stdout is None:
+            raise TypeError(
+                "can't use wait_output() when exec was called with the stdout argument; "
+                "use wait() instead"
+            )
+
         if self._encoding is not None:
             out = io.StringIO()
             err = io.StringIO() if self.stderr is not None else None


Test patch:
diff --git a/test/test_pebble.py b/test/test_pebble.py
index 44e1f7d..6c9b6b7 100644
--- a/test/test_pebble.py
+++ b/test/test_pebble.py
@@ -2925,6 +2925,15 @@ class TestExec(unittest.TestCase):
             ('TXT', '{"command":"end"}'),
         ])
 
+    def test_wait_output_no_stdout(self):
+        stdio, stderr, _ = self.add_responses('123', 0)
+        stdio.receives.append('{"command":"end"}')
+        stderr.receives.append('{"command":"end"}')
+        stdout_buffer = io.BytesIO()
+        process = self.client.exec(["echo", "FOOBAR"], stdout=stdout_buffer, encoding=None)
+        with self.assertRaises(TypeError):
+            process.wait_output()
+
     def test_wait_output_bad_command(self):
         stdio, stderr, _ = self.add_responses('123', 0)
         stdio.receives.append(b'Python 3.8.10\n')
Repository: canonical/operator
Pull request #1001 (2023-08-30 00:12:59)

fix(model): make Secret.set_content invalidate local cache
This is so that if get_content() is called after, it fetches and returns the expected new content. We could also set self._content to the new content, but re-fetching it from Juju seems better.


Patch:
diff --git a/ops/model.py b/ops/model.py
index 906b463..304af8f 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -1225,6 +1225,7 @@ class Secret:
         if self._id is None:
             self._id = self.get_info().id
         self._backend.secret_set(typing.cast(str, self.id), content=content)
+        self._content = None  # invalidate cache so it's refetched next get_content()
 
     def set_info(self, *,
                  label: Optional[str] = None,


Test patch:
diff --git a/test/test_model.py b/test/test_model.py
index d74dd17..8b64347 100755
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -3116,6 +3116,24 @@ class TestSecretClass(unittest.TestCase):
         self.assertEqual(fake_script_calls(self, clear=True),
                          [['secret-get', 'secret:z', '--format=json']])
 
+    def test_set_content_invalidates_cache(self):
+        fake_script(self, 'secret-get', """echo '{"foo": "bar"}'""")
+        fake_script(self, 'secret-set', """exit 0""")
+
+        secret = self.make_secret(id='z')
+        old_content = secret.get_content()
+        self.assertEqual(old_content, {'foo': 'bar'})
+        secret.set_content({'new': 'content'})
+        fake_script(self, 'secret-get', """echo '{"new": "content"}'""")
+        new_content = secret.get_content()
+        self.assertEqual(new_content, {'new': 'content'})
+
+        self.assertEqual(fake_script_calls(self, clear=True), [
+            ['secret-get', 'secret:z', '--format=json'],
+            ['secret-set', 'secret:z', 'new=content'],
+            ['secret-get', 'secret:z', '--format=json'],
+        ])
+
     def test_peek_content(self):
         fake_script(self, 'secret-get', """echo '{"foo": "peeked"}'""")
Repository: matrixorigin/matrixone
Pull request #4252 (2022-07-28 08:53:47)

logservice: fixed duplicated shard metadata record
**What type of PR is this?**

- [ ] API-change
- [x] BUG
- [ ] Improvement
- [ ] Documentation
- [ ] Feature
- [ ] Test and CI
- [ ] Code Refactoring

**Which issue(s) this PR fixes:**

issue #

**What this PR does / why we need it:**

fixed duplicated shard metadata record

**Special notes for your reviewer:**

Not Available

**Additional documentation (e.g. design docs, usage docs, etc.):**

Not Available



Patch:
diff --git a/pkg/logservice/store_metadata.go b/pkg/logservice/store_metadata.go
index ee8ca1ffa..1c4ef6493 100644
--- a/pkg/logservice/store_metadata.go
+++ b/pkg/logservice/store_metadata.go
@@ -258,6 +258,13 @@ func (l *store) addMetadata(shardID uint64, replicaID uint64) {
 	l.mu.Lock()
 	defer l.mu.Unlock()
 
+	for _, rec := range l.mu.metadata.Shards {
+		if rec.ShardID == shardID && rec.ReplicaID == replicaID {
+			plog.Infof("addMetadata for shardID %d skipped, dupl shard", shardID)
+			return
+		}
+	}
+
 	l.mu.metadata.Shards = append(l.mu.metadata.Shards, rec)
 	l.mustSaveMetadata()
 }


Test patch:
diff --git a/pkg/logservice/store_metadata_test.go b/pkg/logservice/store_metadata_test.go
index 8f087df5d..148fa2baa 100644
--- a/pkg/logservice/store_metadata_test.go
+++ b/pkg/logservice/store_metadata_test.go
@@ -41,6 +41,18 @@ func TestAddMetadata(t *testing.T) {
 	assert.Equal(t, uint64(1), ss.mu.metadata.Shards[0].ReplicaID)
 }
 
+func TestAddMetadataRejectDupl(t *testing.T) {
+	cfg := getStoreTestConfig()
+	defer vfs.ReportLeakedFD(cfg.FS, t)
+	cfg.Fill()
+	s := store{cfg: cfg}
+	require.NoError(t, mkdirAll(s.cfg.DataDir, cfg.FS))
+	s.addMetadata(10, 1)
+	s.addMetadata(10, 1)
+	s.addMetadata(10, 1)
+	require.Equal(t, 1, len(s.mu.metadata.Shards))
+}
+
 func TestRemoveMetadata(t *testing.T) {
 	cfg := getStoreTestConfig()
 	defer vfs.ReportLeakedFD(cfg.FS, t)
Repository: matrixorigin/matrixone
Pull request #4255 (2022-07-28 09:37:49)

logservice: fixed dragonboat.ErrInvalidDeadline handling
**What type of PR is this?**

- [ ] API-change
- [x] BUG
- [ ] Improvement
- [ ] Documentation
- [ ] Feature
- [ ] Test and CI
- [ ] Code Refactoring

**Which issue(s) this PR fixes:**

issue #

**What this PR does / why we need it:**

fixed dragonboat.ErrInvalidDeadline handling

**Special notes for your reviewer:**

Not Available

**Additional documentation (e.g. design docs, usage docs, etc.):**

Not Available



Patch:
diff --git a/pkg/logservice/errors.go b/pkg/logservice/errors.go
index d298f408c..7ba4a2494 100644
--- a/pkg/logservice/errors.go
+++ b/pkg/logservice/errors.go
@@ -39,8 +39,8 @@ var errorToCodeMappings = getErrorToCodeMapping()
 func getErrorToCodeMapping() []errorToCode {
 	return []errorToCode{
 		{dragonboat.ErrTimeout, pb.Timeout, true},
-		// TODO: why ErrTimeoutTooSmall is possible
 		{dragonboat.ErrTimeoutTooSmall, pb.Timeout, false},
+		{dragonboat.ErrInvalidDeadline, pb.Timeout, false},
 		{dragonboat.ErrPayloadTooBig, pb.InvalidPayloadSize, true},
 		{dragonboat.ErrRejected, pb.Rejected, true},
 		{dragonboat.ErrShardNotReady, pb.ShardNotReady, true},


Test patch:
diff --git a/pkg/logservice/errors_test.go b/pkg/logservice/errors_test.go
index d5cc54ac2..20acf1149 100644
--- a/pkg/logservice/errors_test.go
+++ b/pkg/logservice/errors_test.go
@@ -24,6 +24,15 @@ import (
 	pb "github.com/matrixorigin/matrixone/pkg/pb/logservice"
 )
 
+func TestTimeoutError(t *testing.T) {
+	code, _ := toErrorCode(dragonboat.ErrTimeout)
+	assert.Equal(t, pb.Timeout, code)
+	code, _ = toErrorCode(dragonboat.ErrTimeoutTooSmall)
+	assert.Equal(t, pb.Timeout, code)
+	code, _ = toErrorCode(dragonboat.ErrInvalidDeadline)
+	assert.Equal(t, pb.Timeout, code)
+}
+
 func TestErrorConversion(t *testing.T) {
 	mappings := getErrorToCodeMapping()
 	for _, rec := range mappings {