Azure: rework how conflict detection works

[JAVGan]

This commit changes the existing code of
AzureService.ensure_can_publish to behave as follows:

1. It no longer retries on error but simply raises whenever a conflict is detected

2. It no longer searches only for "live" or "preview" but for all existing submissions

3. It improves the logging mechanism to inform which state the submission was and its target.

Rationale:

1. If we simply wait for the publishing to finish as the previous version (using retry) we could end up in a scenario where the changes made by the library are no longer the latest, causing the Graph API to return the error: The submission cannot be pushed to as its not the latest.
   With that, we would need to retry the whole publish operations so it makes more sense to just raise ConflictError and let pubtools retry when this error is caught.

2. The previous code were not able to catch a lot of conflicts, resulting in many errors with An In Progress submission already exists
   By looking into each submission target we may caught conflicts early.

3. Logs will now show exactly what was the reason it was already in progress and for which submission target.

4. A retry can/will be implemented on pubtools-marketplacesvm on Azure push in case a ConflictError is detected.

5. Replaces SPSTRAT-549: Azure: Retry publishing when a submission is in progress/conflict #119

Refers to SPSTRAT-611 and SPSTRAT-549

Summary by Sourcery

Rework AzureService.ensure_can_publish to detect conflicts across all submission targets, remove retry logic, raise a dedicated ConflictError on in-progress submissions, and update tests and logging accordingly.

New Features:

• Introduce ConflictError exception for submission conflicts

Enhancements:

• Improve logging in ensure_can_publish to include submission target, status, and result

Tests:

• Update tests to mock get_submissions and validate conflict detection and ensure_can_publish integration

[sourcery-ai]

Reviewer's Guide

This PR reworks the AzureService.ensure_can_publish method to immediately detect and raise on any in-progress submission by iterating over all submissions, removes its long-running retry behavior, introduces a custom ConflictError exception, and updates tests to mock and assert against the new get_submissions API.

Sequence diagram for updated conflict detection in AzureService.ensure_can_publish

sequenceDiagram
participant AzureService
participant Logger
participant Submission
participant ConflictError
AzureService->>Logger: log.info("Ensuring no other publishing jobs are in progress")
AzureService->>AzureService: get_submissions(product_id)
loop For each Submission
    AzureService->>Submission: Check status
    alt Submission status is not "completed"
        AzureService->>Logger: log.error(conflict message)
        AzureService->>ConflictError: raise ConflictError(conflict message)
    end
end

Loading

Class diagram for ConflictError addition and AzureService changes

classDiagram
class AzureService {
  +ensure_can_publish(product_id: str)
  -get_submissions(product_id)
}
class ConflictError {
  "Report a submission conflict error."
}
AzureService ..> ConflictError : raises

Loading

File-Level Changes

Change	Details	Files
Rewrite ensure_can_publish to detect conflicts immediately	Removed retry decorator and old two-state loop Replaced get_submission_state calls with iteration over get_submissions Changed exception from RuntimeError to ConflictError and updated docstring Enhanced logging to report targetType, status, and result	cloudpub/ms_azure/service.py
Add ConflictError exception	Introduced ConflictError subclass of RuntimeError in error module	cloudpub/error.py
Refactor tests for new conflict detection API	Replaced mocks of get_submission_state with get_submissions Removed retry configuration in tests Updated test data to cover all submission targets (draft/preview/live) Adjusted assertion logic for single get_submissions call and error messages Added ensure_can_publish mocks in publish tests	tests/ms_azure/test_service.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• Consider filtering out non‐blocking statuses (e.g. failed or canceled) in ensure_can_publish so that only truly in‐progress submissions raise a ConflictError, avoiding false positives.
• To improve maintainability and testability, extract the iteration and conflict-detection logic in ensure_can_publish into a small helper method.
• Querying all submissions on every publish could get expensive at scale—consider adding server-side filters or pagination to limit the results.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider filtering out non‐blocking statuses (e.g. failed or canceled) in ensure_can_publish so that only truly in‐progress submissions raise a ConflictError, avoiding false positives.
- To improve maintainability and testability, extract the iteration and conflict-detection logic in ensure_can_publish into a small helper method.
- Querying all submissions on every publish could get expensive at scale—consider adding server-side filters or pagination to limit the results.

## Individual Comments

### Comment 1
<location> `tests/ms_azure/test_service.py:752-753` </location>
<code_context>

-    @pytest.mark.parametrize("target", ["preview", "live"])
-    @mock.patch("cloudpub.ms_azure.AzureService.get_submission_state")
+    @mock.patch("cloudpub.ms_azure.AzureService.get_submissions")
     def test_ensure_can_publish_success(
         self,
-        mock_getsubst: mock.MagicMock,
</code_context>

<issue_to_address>
**suggestion (testing):** Missing test for error conditions in ensure_can_publish.

Add a test to confirm that ensure_can_publish raises ConflictError when the submission status is not 'completed', such as 'running' or 'pending'.
</issue_to_address>

### Comment 2
<location> `tests/ms_azure/test_service.py:758-767` </location>
<code_context>
+        submissions = [
</code_context>

<issue_to_address>
**suggestion (testing):** Test does not cover edge cases for get_submissions return values.

Add tests for cases where get_submissions returns an empty list, None, or submissions with unexpected status or result values to ensure ensure_can_publish is robust.

Suggested implementation:

```python
        submissions = [
            {
                "$schema": "https://product-ingestion.azureedge.net/schema/submission/2022-03-01-preview2",  # noqa: E501
                "id": "submission/ffffffff-ffff-ffff-ffff-ffffffffffff/0",
                "product": "product/ffffffff-ffff-ffff-ffff-ffffffffffff",
                "target": {"targetType": tgt},
                "lifecycleState": "generallyAvailable",
                "status": "completed",
                "result": "succeeded",
                "created": "2024-07-04T22:06:16.2895521Z",
            }
        ]

def test_ensure_can_publish_empty_list(monkeypatch):
    def mock_get_submissions(*args, **kwargs):
        return []
    monkeypatch.setattr("ms_azure.service.get_submissions", mock_get_submissions)
    # Replace with actual call to ensure_can_publish and expected assertion
    # Example:
    # assert not ensure_can_publish(...)

def test_ensure_can_publish_none(monkeypatch):
    def mock_get_submissions(*args, **kwargs):
        return None
    monkeypatch.setattr("ms_azure.service.get_submissions", mock_get_submissions)
    # Replace with actual call to ensure_can_publish and expected assertion
    # Example:
    # assert not ensure_can_publish(...)

def test_ensure_can_publish_unexpected_status(monkeypatch):
    def mock_get_submissions(*args, **kwargs):
        return [
            {
                "status": "failed",
                "result": "unknown"
            }
        ]
    monkeypatch.setattr("ms_azure.service.get_submissions", mock_get_submissions)
    # Replace with actual call to ensure_can_publish and expected assertion
    # Example:
    # assert not ensure_can_publish(...)

def test_ensure_can_publish_unexpected_result(monkeypatch):
    def mock_get_submissions(*args, **kwargs):
        return [
            {
                "status": "completed",
                "result": "unexpected_value"
            }
        ]
    monkeypatch.setattr("ms_azure.service.get_submissions", mock_get_submissions)
    # Replace with actual call to ensure_can_publish and expected assertion
    # Example:
    # assert not ensure_can_publish(...)

```

You will need to:
- Replace the comments in each test with the actual call to `ensure_can_publish` and the expected assertion, based on your implementation.
- Ensure that the `monkeypatch` target string matches the import path for `get_submissions` in your codebase.
- If you use fixtures or a different mocking strategy, adapt the test setup accordingly.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[JAVGan]

@lslebodn @ashwgit PTAL

I've closed #119 in favor of this one.

The idea here is that we raise ConflictError whenever any submission target (draft, preview, live) is running and let the pubtools perform the retry in order to avoid the issue of The submission cannot be pushed to as its not the latest

[JAVGan]

Example of the idea: https://github.com/release-engineering/pubtools-marketplacesvm/pull/125/files

[lslebodn]

Could we create similar "wait method" as we have for AWS (AWSProductService.wait_active_changesets)?
*

cloudpub/cloudpub/aws/service.py

Lines 283 to 307 in 075853a

	def wait_active_changesets(self, entity_id: str) -> None:
	"""
	Get the first active changeset, if there is one, and wait for it to finish.
	Args:
	entity_id (str)
	The Id of the entity to wait for active changesets
	"""
	def changeset_not_complete(change_set_list: List[ListChangeSet]) -> bool:
	if change_set_list:
	self.wait_for_changeset(change_set_list[0].id)
	return True
	else:
	return False
	r = Retrying(
	stop=stop_after_attempt(self.wait_for_changeset_attempts),
	retry=retry_if_result(changeset_not_complete),
	)
	try:
	r(self.get_product_active_changesets, entity_id)
	except RetryError:
	self._raise_error(Timeout, f"Timed out waiting for {entity_id} to be unlocked")

[JAVGan]

We can, yes, but I fear we would end up having some errors like "The submission cannot be pushed to as its not the latest" due to the changes we're attempting to publish not being the latest, or other weird errors like "attempting to remove an image which is already published" due to missing the version which was added in parallel, thus I thought on doing the whole operation again by retrying on pubtools.

[JAVGan]

I'm thinking here: what I can do, instead, is wait for it before the whole "main" operation starts, like the first thing to do would be wait and then we do everything. Do you think it would be better? I can open a separate PR for that as well 🙂

[JAVGan]

I was thinking on opening a different PR but I believe I can reuse this one with a commit on top of it. I had a different idea on how to take advantage of this current implementation in order to implement the "wait" feature.

I'll patch it soon

[JAVGan]

Patched.

[JAVGan]

@lslebodn PTAL again.

Now the workflow is:

1. wait for active submissions
2. Look up for SAS / make changes
3. Ensure can publish (raise instead of retrying here)
4. Publish preview/live