Problem streaming legacy FLAC files with IDv3 tags in Firefox

Version
cloned from master and tested with modified example demo, also with 4.24.1 served with https://github.com/navidrome/navidrome
Description
Legacy FLAC files, eg those ripped with EAC before Vorbis comments, do not play with react-music-player in Firefox.  They play fine in chrome, but not in Firefox.  Tested v88.0.1 and v89.0 on Linux, Windows and macOS.
The situation with the files is well described here [the bug report itself is not relevant but the description of the issue and motivation is illustrative]:
https://github.com/dhowden/tag/issues/58
If I use dev tools->Network in Firefox, I can see that Content-Type is correctly set, eg audio/flac in navidrome (changing to audio/x-flac in the included "demo" example with a mini-media server on the side does not help).
BUT the Firefox "type" column says "mpeg".
Navidrome adds "X-Content-Type-Options: nosniff", but still the media does not play.
If I directly enter the media URL into a firefox window, it plays fine with the inbuilt media player!  In such a case, the "type" column says "flac" or "unknown", but not the disastrously wrong "mpeg".  Somehow, react-music-player is causing Firefox to misinterpret the media type.
Note that while such .flac media files are correctly identified by ffprobe, regular Linux file command incorrectly calls them MPEG-1 Layer 3 with ID3v2
Steps
Find a legacy FLAC file with ID3v2 tags in the beginning.  An example can be found here:
https://github.com/dhowden/tag/files/3873999/01.-.Lookee.Here-trimmed.zip
I can provide a full track from a legacy EAC session if helpful, but prefer to do that directly to the author, not publicly.
Either serve the file in navidrome, or quickly and easily to test, replace one of the "musicSrc" links in the example/example.js file in the player demo.
$ npm run demo

On chrome, everything is fine.
On firefox, you will see that:

The file might be skipped if you added it in a multi-element playlist eg audioList1
The file "hangs" with a spinning pinwheel and the album art does not rotate, in a single-element playlist like audioList2.  This hanging/stuck artwork is the observed navidrome behavior.

The Firefox javascript error console shows audio decode errors:
Media resource http://server:port/01%20-%20Lookee%20Here-trimmed.flac could not be decoded.
Media resource http://server:port/01%20-%20Lookee%20Here-trimmed.flac could not be decoded, error: Error Code: NS_ERROR_DOM_MEDIA_METADATA_ERR (0x806e0006)

...i just tried these other webplayers

Airsonic Refix
Jamstash
Subfire

and they all behave similarly, so ... some combination of however these players work, Firefox and legacy FLAC is bad...


mmseqs taxonomy Error: orf filter prefilter died / Error: Search died

Expected Behavior
k-mer similarity threshold: 145
Starting prefiltering scores calculation (step 1 of 1)
Query db start 1 to 469186
Target db start 1 to 579233
[=================================================================] 100.00% 469.19K 3s 382ms
6.094516 k-mers per position
435 DB matches per sequence
0 overflows
0 queries produce too many hits (truncated result)
0 sequences passed prefiltering per query sequence
0 median result list length
464221 sequences with 0 size result lists
Time for merging to orfs_pref: 0h 0m 0s 75ms
munmap_chunk(): invalid pointer
tmp/12438089539006473617/taxonomy_tmp/18301032544654743241/taxpercontig.sh: line 66: 3001403 Aborted                 (core dumped) $RUNNER "$MMSEQS" prefilter "${ORFS_DB}" "${TARGETDB_IDX}" "${TMP_PATH}/orfs_pref" ${ORF_FILTER_PREFILTER}
Error: orf filter prefilter died
Error: Search died
Steps to Reproduce (for bugs)
mmseqs  easy-taxonomy  -s 7.5  C10.fasta ../db/refseq-prot/mmseqs/refseq_viral  result tmp --lca-mode 2
Context
I try to annotate virus contig with refseq database uing mmseqs  easy-taxonomy, I get errors below, When  I use the same command with tmp files, I can get the taxonomy annotation files.
But I use '--orf-filter 0', I get no errors.
Your Environment
mmseqs  version
45111b641859ed0ddd875b94d6fd1aef1a675b7e
mmseqs2-sse4-r13

I encountered the orf filter prefilter died issue today. I suspect it might be due to insufficient memory available to the program, even though there was no indication of memory shortage. According to the MMseqs2 documentation, the program should automatically analyze the available memory and split the data accordingly to meet memory requirements. However, I suspect the program did not correctly identify the maximum memory I allocated for my task submitted to HPC (via qsub), and it may have mistakenly considered the total available memory in HPC as the memory accessible for the current task. I resolved this issue by adding the parameter --split-memory-limit 80G. Hope this helps you.


Release.Deployment.StartTime

Why Release.Deployment.StartTime is not being included in here? Is in the logs as RELEASE_DEPLOYMENT_STARTTIME. Is it planned to stop declaring this variable or is just missing in the documentation?

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

ID: 611642f0-881f-10b9-ae94-3c562001d1f0
Version Independent ID: 41efb94b-4c7a-64a3-b23b-70c8634ae727
Content: Release variables in Azure Pipelines - Azure Pipelines & TFS
Content Source: docs/pipelines/release/variables.md
Product: devops
GitHub Login: @alexhomer1
Microsoft Alias: ahomer

Scratch that... I see that only a few variables are Included in the documentation as per disclaimer: "Some of the most significant variables are described in the following tables."

Created Pull Request #2544 to clarify even further that the tables in docs/pipelines/release/variables.md do not contain the fully up to date list of variables, but only "Some of the most significant".

Closing PR #2544 due to automated validation failures... Opened PR #2546 : same validation errors

Thank you for your feedback. I will update the topic to include your recommendations.

Is there a "Full" list of variable available ?


Nativescript Angular: Cant use (focus) on IOS

Hey Guys,
I am trying to use the (focus) functionality of textfield on the MaskedTextField component. This works for android but not for IOS. We are still on nativescript 3.4 and maybe this is fixed in the new version of maskedTextField, im not sure. But for us its still broken.
Hope you guys van help us out with this.
Kind regards,
BT

Hey @btaluy , I do not understand what are you trying to do and you do not have any sample code, so not much I can do. It is best you create a sample on the playgorund that demonstrates the problem.
Note that the iOS implementation has a specific implementation that the cursor goes in the beginning of the field when the field is focused:
https://github.com/PeterStaev/nativescript-masked-text-field/blob/8d800f58631a4a408fb40420ca4464bfea4e792b/masked-text-field.ios.ts#L99-L101

I'm trying to use the (focus) event on your plugin, since it extends the textField component it should work. The (blur) event works fine with it on IOS. try to use (focus) on your component in nativescript angular, you will see that the function which is assigned to the focus event does not get triggered.

Hey @btaluy , I think I found the problem and it should be fixed in the new version on npm.


[Calling] PPTLive Component

What

Add the PPTLive Component Level logic

Why

How Tested

Process & policy checklist

[ ] I have updated the project documentation to reflect my changes if necessary.
[ ] I have read the CONTRIBUTING documentation.

Is this a breaking change?

[ ] This change causes current functionality to break.

Can you complete the PR description template also, for UI components having screenshots in the How Tested section lets reviewers know how the component implementation looks visually as its tough to review that via code alone


Dev storagecache microsoft.storage cache 2023 03 01 preview

This takes the amlfilesystem content from the 2021-11-01-preview api for storagecache   Microsoft.StorageCache and adds the new public preview api 2023-03-01-preview version.
This 2023-03-01-preview version is based on storagecache 2022-05-01 version and the previously approved 2021-11-01-preview version's amlfilesystem content.
The amlfilesystem is a new resource that is currently in private preview and this api version will move it to public preview.
The only changes from the preview version of amlfilesystem content was the move to use a separate amlfilesystem.json file instead of adding it to the existing storagecache.json and to remove the management subnet that was in the private preview.
Adding this comment from below to make it more visible.
One question is on the comments to storagecache.json. That file has been that way for a few years and I think just about any change to that will be a breaking change against the existing service.
I squashed the fixes and made sure that the baseline was applied first which as from the 2022-05-01 GA version.
Then I added the approved GA 2023-01-01 version that is not merged yet as the next base.
After that the only real changes I applied to storagecache.json are from an approved 2022-09-01-preview spec in the private preview repo. That just included a new scalingFactor field.
The main content is the amlfilesystem.json. This was a new resource from the 2021-11-01-preview spec in the private repo.
It wasn't directly ported over from there because I made it its own file for public preview to help simplify reviews between existing content and new content.
stroagecache.json = GA Merged 2022-05-01 + GA Approved 2023-03-01 + Private Preview 2022-09-01-preview
amlfilesystem.json = Private Preview 2021-11-01-preview
This change for 2023-03-01-preview is the public preview of the 2 private previews for both storagecache and amlfilesystem.
Private Preview 2022-09-01-preview Pull Request
https://github.com/Azure/azure-rest-api-specs-pr/pull/7982
Private Preview 2021-11-01-preview Pull Request
https://github.com/Azure/azure-rest-api-specs-pr/pull/2097
Approved, not merged until RP rollout 2023-01-01
https://github.com/Azure/azure-rest-api-specs/pull/22135
MSFT employees can try out our new experience at OpenAPI Hub  - one location for using our validation tools and finding your workflow.

Changelog
Add a changelog entry for this PR by answering the following questions:

What's the purpose of the update?

[ ] new service onboarding
[x] new API version
[ ] update existing version for new feature
[ ] update existing version to fix swagger quality issue in s360
[ ] Other, please clarify

When are you targeting to deploy the new service/feature to public regions? Please provide the date or, if the date is not yet available, the month.   2022-10.
When do you expect to publish the swagger? Please provide date or, the the date is not yet available, the month. 2022-11
If updating an existing version, please select the specific language SDKs and CLIs that must be refreshed after the swagger is published.

[ ] SDK of .NET (need service team to ensure code readiness)
[ ] SDK of Python
[ ] SDK of Java
[ ] SDK of Js
[ ] SDK of Go
[ ] PowerShell
[ ] CLI
[ ] Terraform
[ ] No refresh required for updates in this PR

Contribution checklist:

[x] I commit to follow the Breaking Change Policy of "no breaking changes"
[x] I have reviewed the documentation for the workflow.
[x] Validation tools were run on swagger spec(s) and errors have all been fixed in this PR. How to fix?

If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist

Applicability: :warning:
If your changes encompass only the following scenarios, you should SKIP this section, as these scenarios do not require ARM review.

Change to data plane APIs
Adding new properties
All removals

Otherwise your PR may be subject to ARM review requirements. Complete the following:

[x] Check this box if any of the following apply to the PR so that the label "ARMReview" and "WaitForARMFeedback" will be added by bot to kick off ARM API Review.  Missing to check this box in the following scenario may result in delays to the ARM manifest review and deployment.

Adding a new service
Adding new API(s)
Adding a new API version
-[ ] To review changes efficiently, ensure you are using OpenAPIHub to initialize the PR for adding a new version. More details, refer to the wiki.

[x] Ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.

[ ] If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.

Breaking Change Review Checklist
If any of the following scenarios apply to the PR, request approval from the Breaking Change Review Board as defined in the Breaking Change Policy.

[ ] Removing API(s) in a stable version
[ ] Removing properties in a stable version
[ ] Removing API version(s) in a stable version
[ ] Updating API in a stable or public preview version with Breaking Change Validation errors
[ ] Updating API(s) in public preview over 1 year (refer to Retirement of Previews)

Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.
Please follow the link to find more details on PR review process.

/azp run unifiedPipeline

/azp run unifiedPipeline

},

Common parameters best practice - use common types
There are definitions of common parameters in commontypes types.json that you can use by $ref instead of declaring your own, like SubscriptionIdParameter, OperationIdParameter, and ApiVersionParameter,
See
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L630

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:3497 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

This looks like its based on the standard ARM SKUs contract? If so I wonder if we can get this included in common-types definitions

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:3101 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

I think I see you using this to declare resource ids - but resource ids aren't really fully qualified URLs - they are just absolute paths

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:3145 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

"/subscriptions/{subscriptionId}/providers/Microsoft.StorageCache/locations/{location}/ascOperations/{operationId}": {

Are you using this to implement the Azure-AsyncOperation header pattern? I think the usual name for this resource type would be something like operationsStatus.

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:161 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

          "$ref": "#/definitions/CloudError"

Error responses - use common types
There are definitions of error responses and details in commontypes types.json that you can use instead of declaring your own.
See e.g. ErrorResponse
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L300
And ErrorDetail
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#LL258C21-L258C21

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:198 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

      },

Location parameters - use common types
There are definitions of common parameters in commontypes types.json that you can use instead of declaring your own, like LocationParameter, which is useful for modeling locations/{locationName} in paths like ARM location-based routing scenarios.
See https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L657

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:179 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

      }

Best practice - OperationId parameters - Common parameters best practice - use common types
There are definitions of common parameters in commontypes types.json that you can use by $ref instead of declaring your own, like SubscriptionIdParameter, OperationIdParameter, and ApiVersionParameter,
See
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L630

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:186 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

"/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.StorageCache/caches/{cacheName}/spaceAllocation": {

post actions should be a verb. E.g. 'getSpaceAllocation' or 'changeAllocation' or just 'reallocate'
But also, I wonder if you could model this as a PUT or PATCH action on the cache, since it seems to be a pretty straightforward ideally idempotent 'write' or 'update' to the cache description (plus some side effects).

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1729 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

Best practice - don't declare these yourself, just reuse common-types
OperationListResult - use common types
The OperationListResult type is defined correctly in commontypes types.json. It is recommended to reference and reuse the common definition instead of declaring your own. [Otherwise it is frequently declared incorrectly.]
            "schema": {
              "$ref": "../../../../../common-types/resource-management/v3/types.json#/definitions/OperationListResult"
            }

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1950 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

Best practice - don't declare these yourself, just reuse common types
Error responses - use common types
There are definitions of error responses and details in commontypes types.json that you can use instead of declaring your own.
See e.g. ErrorResponse
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L300
And ErrorDetail
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#LL258C21-L258C21

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1987 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

Best practice - don't declare these yourself, just reuse common types's OperationStatusResult:
https://github.com/Azure/azure-rest-api-specs/blob/main/specification/common-types/resource-management/v3/types.json#L418

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2036 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

    },

Best practice - Instead of declaring all these, inherit (allOf) common-types TrackedResource.
Instead of defining your own resource class with required top level properties, 'id', 'type', 'name', and 'location' and 'tags' (for TrackedResource) its recommended to inherit from ProxyResource or TrackedResource in common-types v2 or v3.json. (The v2 definition does NOT include systemData, and is otherwise similar to the v3 one).
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L56

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2092 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

            "Cancelled",

spell with one L, to match the RPC spec

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2130 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

          "description": "Array of IP addresses that can be used by clients mounting this Cache.",

Are both IPv4 and IPv6 supported?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2118 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        },

'zones' is supposed to be at top level of the resource payload - not inside of 'properties'!

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2185 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

best practice - reuse definitions from common types managedidentity.json
UserAssignedIdentity, ManagedIdentity - use common types
Instead of defining your own managedIdentity or userAssignedIdentity class, I highly recommend finding one to use by $ref under common-types such as
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v5/managedidentity.json#L50

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2268 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "password": {

+x-ms-mutability: "create" or create and update ?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2398 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "password"

If you actually require password sent in every update request, it could be more difficult for everyone to call your API unless they fetch the password every time.
I'd suggest to make updating the password optional, and only react when a value is actually provided

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2406 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

    },

ARM recommends enums over booleans as a best practice.
Standard guidance is: replace boolean/switch properties with better enum
A boolean will forever have two valid values (true or false). A string enum type is always preferred. Also, properties should always provide better values just than True and False. For example two switches "isTypeA" and "isTypeB" should be replaced with one enum "type": [A, B, DefaultType]. Even if you still believe [True, False] are the correct values for a property, you should use a string enum with values [True, False] instead of boolean. Enums are always a more flexible and future proof option because they allow additional values to be added in the future in a non-breaking way, e.g. [True, False, Unknown].

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2468 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "Error"

Is just 'Error' enough information for people to debug?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2480 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "bindPassword": {

+x-ms-mutability?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2495 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "bindDn": {

is Dn an obvious abbreviation or should we call this 'bindDistinguishedName'?
I couldn't understand it without also reading the description.

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2491 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

          "description": "The Bind password to be used in the secure LDAP connection. This value is stored encrypted and not returned on response."

Encrypted or hashed?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2498 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

    },

Would anyone really want to set this false, in this modern age?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2460 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

    "ldapBaseDN": {

is Dn an obvious abbreviation or should we call this 'ldapBaseDistinguishedName'?
I probably couldn't understand it without also reading the description.

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2453 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "bindDn": {

PS, the capitalization should be 'DN' if you decide to stick with DN.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2491 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

    },

ARM recommends enums over booleans as best practice
Standard guidance is: replace boolean/switch properties with better enum
A boolean will forever have two valid values (true or false). A string enum type is always preferred. Also, properties should always provide better values just than True and False. For example two switches "isTypeA" and "isTypeB" should be replaced with one enum "type": [A, B, DefaultType]. Even if you still believe [True, False] are the correct values for a property, you should use a string enum with values [True, False] instead of boolean. Enums are always a more flexible and future proof option because they allow additional values to be added in the future in a non-breaking way, e.g. [True, False, Unknown].

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2571 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

  "description": "Describes a reference to Key Vault Key.",

Please check up on how we word + capitalize such things in KeyVault docs, in case this is not consistent

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2588 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

  "description": "Describes a reference to Key Vault Key.",

And/or consult cloud style guide

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2588 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

  "description": "Result of the request to list Caches. It contains a list of Caches and a URL link to get the next set of results.",

I'd suggest not capitalizing caches everywhere unless maybe you're capitalizing 'Storage Cache' everywhere. Check with your cloud / writing style experts.

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2611 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

      }

setting type: string and modelAsString sounds a little contradictory, does that work?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2245 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

      "default": 1500

I don't see why you limit maximum MTU to 1500, can't it be much higher sometimes?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2279 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "Down",

What is the difference?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2637 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

  "x-ms-azure-resource": true

Please inherit allOf common-types/v3/types.json TrackedResource instead of redeclaring it.

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2794 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "Cancelled",

Spell with one L.

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2834 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "password"

(E.g. you can't just GET the resource, change one field, and PUT it back to the server)

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2406 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

    },

ARM recommends avoiding boolean properties, for extensibility.
Standard guidance is: replace boolean/switch properties with better enum
A boolean will forever have two valid values (true or false). A string enum type is always preferred. Also, properties should always provide better values just than True and False. For example two switches "isTypeA" and "isTypeB" should be replaced with one enum "type": [A, B, DefaultType]. Even if you still believe [True, False] are the correct values for a property, you should use a string enum with values [True, False] instead of boolean. Enums are always a more flexible and future proof option because they allow additional values to be added in the future in a non-breaking way, e.g. [True, False, Unknown].

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2732 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

      "format": "int32",

You might want to support floating point, it works better for some unit types.

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:3231 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

Hi, @TimLovellSmith,
Thanks for the comments.  I'll start working on them.
I hadn't addressed all of the tooling checks because they don't seem to run all of the time, so I haven't seen what else has to be fixed since my last update.
One question is on the comments to storagecache.json.  That file has been that way for a few years and I think just about any change to that will be a breaking change against the existing service.
I squashed the fixes and made sure that the baseline was applied first which as from the 2022-05-01 GA version.
Then I added the approved GA 2023-01-01 version that is not merged yet as the next base.
After that the only real changes I applied to storagecache.json are from an approved 2022-09-01-preview spec in the private preview repo.  That just included a new scalingFactor field.
The main content is the amlfilesystem.json.  This was a new resource from the 2021-11-01-preview spec in the private repo.
It wasn't directly ported over from there because I made it its own file for public preview to help simplify reviews between existing content and new content.
stroagecache.json  =  GA Merged 2022-05-01   + GA Approved 2023-03-01 + Private Preview 2022-09-01-preview
amlfilesystem.json =                                                                                Private Preview 2021-11-01-preview
This change for 2023-03-01-preview is the public preview of the 2 private previews for both storagecache and amlfilesystem.
Private Preview 2022-09-01-preview Pull Request
https://github.com/Azure/azure-rest-api-specs-pr/pull/7982
Private Preview 2021-11-01-preview Pull Request
https://github.com/Azure/azure-rest-api-specs-pr/pull/2097
Approved, not merged until RP rollout 2023-01-01
https://github.com/Azure/azure-rest-api-specs/pull/22135
https://github.com/Azure/azure-rest-api-specs/pull/22135

The pipelines still won't run, so I don't know what the remaining errors are.
Last I remember for amfilesystem.json it said I need to flatten the main resource but I had that added x-ms-flatten just like in storagecache.json.
It said I needed name type and id but I also had those and they were readonly just like in storagecache.json.
It said the new field in the new version copied from the private preview was somehow a breaking change.
Then a few others around async.

/azp run

@raych1
There's a few checks that I'm not sure are correct or are providing enough detail for me to fix the problem.
In the breaking changes, its flagging adding new things to the new api version as breaking changes.
There are errors for adding properties, paths, and other content to the new api version.
These shouldn't be breaking changes.
A missing base commit tag was applied.  The first three commits are the base from the openapi hub tool that created this PR.
I am getting the AvoidNestedProperties error even though I am using x-ms-flatten, just like we are in the old storagecache.json file.
Model definition 'AmlFilesystem' must have the properties 'name', 'id' and 'type' in its hierarchy and these properties must be marked as readonly.   I have all of those properties and they are readonly.  Also similar to the old storagecache.json file.
The patch operation body parameter schema should not contains properties name, type, location.    This was approved for amlfilesystem in the private preview version but now its saying that the patch body can't contain those fields.
Thanks,
Brian

Moving to ready for review to see if it will run the checks again.
The only checks I saw were breaking changes for the 2023-01-01 GA commit that is approved but won't be committed for another week or two.
I need to get these private preview apis merged into this public preview version approved so we can finalize the code changes that would correspond to anything that comes up during the review.

@dw511214992 Can you check on why the checks aren't running on this PR.
Also not sure why the NoBaseCommit label is applied when this was started in OpenApiHub and it generated the first three commits.
Thanks.

    "requireValidCertificate": {

Document the default behavior of requireValidCertificate if not specified?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2463 in c302039. [](commit_id = c302039f20fcdf2a12b25877e2bb8f60276c2588, deletion_comment = False)

    "autoDownloadCertificate": {

Document the default behavior of autoDownloadCertificateif not specified?

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2467 in c302039. [](commit_id = c302039f20fcdf2a12b25877e2bb8f60276c2588, deletion_comment = False)

        "Error"

I guess this is breaking vs 2023-01-01.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2480 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "bindPassword": {

This could look breaking, but the bottom line is you should declare the OpenAPI spec in a way which a) matches your service behavior b) said service behavior is secure

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2495 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

    },

I guess this is breaking vs 2023-01-01.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2468 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

Don't block on this feedback.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:3101 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

This is really just calling out documentation you can improve - non breaking.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:3145 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

"/subscriptions/{subscriptionId}/providers/Microsoft.StorageCache/locations/{location}/ascOperations/{operationId}": {

This sounds like it would be breaking vs 2023-01-01.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:161 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

      },

This should be non-breaking in theory.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:179 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

      }

This should be non-breaking in theory.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:186 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

          "$ref": "#/definitions/CloudError"

This should be non-breaking in theory.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:198 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

"/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.StorageCache/caches/{cacheName}/spaceAllocation": {

This may be breaking but it looks like a clear violation of ARM naming guidelines. @rkmanda @JeffreyRichter @suhasrao what should we do here?

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1729 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

          "description": "Array of IP addresses that can be used by clients mounting this Cache.",

(clarify docs)

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2118 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

            "Cancelled",

This may be breaking but its a violation of the RPC spec, which is not normally allowed by ARM. Is ARM compatible with this, or does it need to be fixed? @pilor @rkmanda

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2130 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

This should be non-breaking in theory.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1950 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "bindDn": {

I guess this is breaking vs 2023-01-01.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2491 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "required": true,

This makes sense to me as a bugfix even if it looks breaking.

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:475 in c302039. [](commit_id = c302039f20fcdf2a12b25877e2bb8f60276c2588, deletion_comment = False)

},

This should be non-breaking in theory. YMMV. Please try it and see what the linters detect.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2036 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

    "requireValidCertificate": {

(better documentation is nonbreaking)

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2463 in c302039. [](commit_id = c302039f20fcdf2a12b25877e2bb8f60276c2588, deletion_comment = False)

    },

This should be non-breaking in theory. YMMV. Please try it and see what the linters detect.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2092 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

This should be non-breaking, on-the-wire, in theory. YMMV. Please try it and see what the linters detect.
It may seem to break SDK by e.g. removing the 'AscOperation' type... and replacing it with another type instead. You can probably minimize the pain by continuing to declare and reference the 'AscOperation' type but modelling it as 'allOf' e.g OperationStatusResult.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1987 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

      "default": 1500

(Declaring limits which don't actually always apply could be confusing and may in future break apps which assume you follow the limits.)

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2279 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

ARM won't treat it as a terminal state if the RP actually returns it and will continue to poll.  However, I doubt this RP ever returns this state since I can't find a "/cancel" POST action.

            "Cancelled",

This may be breaking but its a violation of the RPC spec, which is not normally allowed by ARM. Is ARM compatible with this, or does it need to be fixed? @pilor @rkmanda
In reply to:<PHONE_NUMBER>
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2130 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

ARM won't treat it as a terminal state if the RP actually returns it and will continue to poll. However, I doubt this RP ever returns this state since I can't find a "/cancel" POST action.

        "Down",

Just consider adding documentation.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2637 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

  "x-ms-azure-resource": true

Using inheritance should not break most customers AFAIK

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2794 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

        "Cancelled",

Consider this a bugfix. ARM won't recognize it as a terminal state otherwise, and canceled deployments could hang forever. When you add support for Cancelation.

In reply to:<PHONE_NUMBER>

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2834 in 3bd573a. [](commit_id = 3bd573abcfb2dcc84bdfd24b2c8da8699538e442, deletion_comment = False)

},

Common parameters best practice - use common types There are definitions of common parameters in commontypes types.json that you can use by $ref instead of declaring your own, like SubscriptionIdParameter, OperationIdParameter, and ApiVersionParameter, See
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L630
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:3497 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

Pushed a commit to fix this for this first ref change to see if the tools complain about switching refs, in this case for parameters.

"/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.StorageCache/caches/{cacheName}/spaceAllocation": {

This may be breaking but it looks like a clear violation of ARM naming guidelines. @rkmanda @JeffreyRichter @suhasrao what should we do here?
In reply to:<PHONE_NUMBER>
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1729 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

It is certainly a break to change the URL from one thing to another. I think you're suggested changing "spaceAllocation" to asome verb/action? That changes the URL and so it would be breaking.

al state if the RP actually returns it and will continue to poll. However, I doubt this RP ever returns this state since I can't find a "/cancel" POST action.

Since cancelled is an existing enum value but never used, I can either change it, leave it alone, or remove it.
Not sure which is better.
That enum in our model comes from the ResourceProvider.API library.

          "$ref": "#/definitions/CloudError"

Error responses - use common types There are definitions of error responses and details in commontypes types.json that you can use instead of declaring your own. See e.g. ErrorResponse
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L300
And ErrorDetail
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#LL258C21-L258C21
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:198 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

In the case where the fields match but the definitions have different names will it be a breaking change?
I wasn't sure if different sdks generate the class names using the definition names and if there would be a conflict between these types if I switch to a new version.
We have definitions for CloudError that lines up with common type ErrorResponse.
They both have an error property with different definitions CloudeErrorBody or ErrorDetail.  These also match except for the
ErrorDetail contains an addition field.
"additionalInfo": {
"readOnly": true,
"type": "array",
"items": {
"$ref": "#/definitions/ErrorAdditionalInfo"
},
"x-ms-identifiers": [],
"description": "The error additional info."
}

"/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.StorageCache/caches/{cacheName}/spaceAllocation": {

This may be breaking but it looks like a clear violation of ARM naming guidelines. @rkmanda @JeffreyRichter @suhasrao what should we do here?
In reply to:<PHONE_NUMBER>
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1729 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

It is certainly a break to change the URL from one thing to another. I think you're suggested changing "spaceAllocation" to asome verb/action? That changes the URL and so it would be breaking.

@JeffreyRichter would a good compromise be to bring in a new action verb, in addition to the old one, and mark the old one as deprecated?

"/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/Microsoft.StorageCache/caches/{cacheName}/spaceAllocation": {

This may be breaking but it looks like a clear violation of ARM naming guidelines. @rkmanda @JeffreyRichter @suhasrao what should we do here?
In reply to:<PHONE_NUMBER>
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1729 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

It is certainly a break to change the URL from one thing to another. I think you're suggested changing "spaceAllocation" to asome verb/action? That changes the URL and so it would be breaking.

@JeffreyRichter would a good compromise be to bring in a new action verb, in addition to the old one, and mark the old one as deprecated?

I'm lacking a lot of context on this issue. Usually, we recommend NOT introducing 2 ways of accomplishing the same thing as it is just confusing for customers. Also, customers using 1 thing and deprecating it making them move to another thing just to "clean up the API" is seen by customers to be a gratuitous change for no value/benefit to them - busy work. So, we usually deem it best to live with the blemishes.
So, to make this kind of gratuitous break, there would have to be a fair some great justification.

@JeffreyRichter
For breaking changes, the list in the checks for this api are inherited from the changes that are merged into this api version.
https://github.com/Azure/azure-rest-api-specs/pull/22135/checks?check_run_id=10704482577 - 2023-01-01 GA
https://github.com/Azure/azure-rest-api-specs-pr/pull/7982/checks?check_run_id=9480440998 - 2022-09-01 private preview
I'm going through the new comments now and doing what I can to work through the comments without additional breaking changes.  The only one that should show up is the change from Cancelled to Canceled to match the ARM spec for provisioningState in stroagecache.json.   It wasn't part of the codes enum so it can never be returned.
For the new amlfilesystem.json I remove the value from the enum.
I'll still fill out a breaking change form and attend the meeting on Monday.
Thanks,
Brian

},

I think I see you using this to declare resource ids - but resource ids aren't really fully qualified URLs - they are just absolute paths
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:3145 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

Fixed. It was a bad description.  It's just the resource name.

      },

Best practice - Location parameters - use common types There are definitions of common parameters in commontypes types.json that you can use instead of declaring your own, like LocationParameter, which is useful for modeling locations/{locationName} in paths like ARM location-based routing scenarios. See
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L657
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:179 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

Fixed.

      }

Best practice - OperationId parameters - Common parameters best practice - use common types There are definitions of common parameters in commontypes types.json that you can use by $ref instead of declaring your own, like SubscriptionIdParameter, OperationIdParameter, and ApiVersionParameter, See
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L630
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:186 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

Fixed.

},

Best practice - don't declare these yourself, just reuse common-types
OperationListResult - use common types The OperationListResult type is defined correctly in commontypes types.json. It is recommended to reference and reuse the common definition instead of declaring your own. [Otherwise it is frequently declared incorrectly.]
            "schema": {
              "$ref": "../../../../../common-types/resource-management/v3/types.json#/definitions/OperationListResult"
            }

Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:1950 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

Our equivalent to this is almost the same but it also includes logSpecifications and metricsSpecifications fields.

    },

Best practice - Instead of declaring all these, inherit (allOf) common-types TrackedResource.
Instead of defining your own resource class with required top level properties, 'id', 'type', 'name', and 'location' and 'tags' (for TrackedResource) its recommended to inherit from ProxyResource or TrackedResource in common-types v2 or v3.json. (The v2 definition does NOT include systemData, and is otherwise similar to the v3 one).
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L56
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2092 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

Updated the new amlfilesystem to use TrackedResource.  Didn't update the existing storagecache.

},

Common parameters best practice - use common types There are definitions of common parameters in commontypes types.json that you can use by $ref instead of declaring your own, like SubscriptionIdParameter, OperationIdParameter, and ApiVersionParameter, See
https://github.com/Azure/azure-rest-api-specs/blob/be6cd9ccfcb6ba08c1c206627026eabfbff31fc1/specification/common-types/resource-management/v3/types.json#L630
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:3497 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

Fixed.

There is a breaking change check that showed up pointing out that the ResourceGroupParameter was removed.
The new version is missing a client parameter that was found in the old version. Was 'ResourceGroupParameter' removed or renamed?
Not sure why thats an issue since it had identical fields and other details.

        "Down",

Consider adding documentation about how Down Degraded and Unknown are different from each other and things like Transitioning. Changing the enum would be breaking.
In reply to:<PHONE_NUMBER>
Refers to: specification/storagecache/resource-manager/Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json:2637 in 3bd573a. [](commit_id = 3bd573a, deletion_comment = False)

Additonal notes where added here.

LGTM now

This ARMSignedOff PR still needs needs Breaking Changes Approved Label. I guess, that's why, automation added WaitForARMFeedback label again. Just FYI, @TimLovellSmith, @JeffreyRichter, @brpanask.
Alos, removing ARMWaitForFeedback as the PR is already SignedOff.

@dw511214992
I have some questions about these checks in the lint diff that are failing.
For the additional properties warnings, I don't know what it needed.  It has x-ms-flatten there.  Similar to our private preview version of this and our storagecache.json file.
The Patch Identity and Sku properties aren't modifiable.  So I can't add them to the patch body since its not taking in readonly or createonly values.  I just made the Patch body separate from the PUT body to fix other errors saying Location can't be in the Patch body.
The Long Running Response Statue code must have 200 and 204 for delete.    I have that so I'm not sure why this is an issue.
The 202 was in there during private preview and its deleting a whole cluster behind the scenes for this resource.

There are many breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/19724/checks?check_run_id=10940083018

There are many breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/19724/checks?check_run_id=10940083018

@JeffreyRichter
Yes, These are from the unmerged GA version 2023-01-01 and the private preview 2022-09-01.
Only a few are from some existing issues found during ARM review.
Here is an early comment I left with the details and the PRs with the breaking changes approvals.
For breaking changes, the list in the checks for this api are inherited from the changes that are merged into this api version.
https://github.com/Azure/azure-rest-api-specs/pull/22135/checks?check_run_id=10704482577 - 2023-01-01 GA
https://github.com/Azure/azure-rest-api-specs-pr/pull/7982/checks?check_run_id=9480440998 - 2022-09-01 private preview
I'm going through the new comments now and doing what I can to work through the comments without additional breaking changes. The only one that should show up is the change from Cancelled to Canceled to match the ARM spec for provisioningState in stroagecache.json. It wasn't part of the codes enum so it can never be returned.
For the new amlfilesystem.json I remove the value from the enum.
I'll go through and point out the new ones which I know are Cancelled to Canceled and just a few others that just showed up.
I'll gather them now and update this thread.
I'll still fill out a breaking change form and attend the meeting on Monday.

There are many breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/19724/checks?check_run_id=10940083018

@JeffreyRichter Yes, These are from the unmerged GA version 2023-01-01 and the private preview 2022-09-01. Only a few are from some existing issues found during ARM review.
Here is an early comment I left with the details and the PRs with the breaking changes approvals.
For breaking changes, the list in the checks for this api are inherited from the changes that are merged into this api version. https://github.com/Azure/azure-rest-api-specs/pull/22135/checks?check_run_id=10704482577 - 2023-01-01 GA https://github.com/Azure/azure-rest-api-specs-pr/pull/7982/checks?check_run_id=9480440998 - 2022-09-01 private preview
I'm going through the new comments now and doing what I can to work through the comments without additional breaking changes. The only one that should show up is the change from Cancelled to Canceled to match the ARM spec for provisioningState in stroagecache.json. It wasn't part of the codes enum so it can never be returned. For the new amlfilesystem.json I remove the value from the enum.
I'll go through and point out the new ones which I know are Cancelled to Canceled and just a few others that just showed up. I'll gather them now and update this thread.
I'll still fill out a breaking change form and attend the meeting on Monday.

2022-09-01-preview didn't need breaking change approval. The check passed and in it, it just noted the new field in the new api version and that a ref to a string was replaced with a string which fixed an s360 where x-ms-secret wasn't understood when applied to the ref, but did work directly on the string type.
⚠ 1017 - ReferenceRedirection | The '$ref' property points to different models in the old and new versions.New: Microsoft.StorageCache/preview/2022-09-01-preview/storagecache.json#L3138:9Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L3132:9
💬 1041 - AddedPropertyInResponse | The new version has a new property 'scalingFactor' in response that was not found in the old version.New: Microsoft.StorageCache/preview/2022-09-01-preview/storagecache.json#L1897:11Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L1897:11

There are many breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/19724/checks?check_run_id=10940083018

@JeffreyRichter Yes, These are from the unmerged GA version 2023-01-01 and the private preview 2022-09-01. Only a few are from some existing issues found during ARM review.
Here is an early comment I left with the details and the PRs with the breaking changes approvals.
For breaking changes, the list in the checks for this api are inherited from the changes that are merged into this api version. https://github.com/Azure/azure-rest-api-specs/pull/22135/checks?check_run_id=10704482577 - 2023-01-01 GA https://github.com/Azure/azure-rest-api-specs-pr/pull/7982/checks?check_run_id=9480440998 - 2022-09-01 private preview
I'm going through the new comments now and doing what I can to work through the comments without additional breaking changes. The only one that should show up is the change from Cancelled to Canceled to match the ARM spec for provisioningState in stroagecache.json. It wasn't part of the codes enum so it can never be returned. For the new amlfilesystem.json I remove the value from the enum.
I'll go through and point out the new ones which I know are Cancelled to Canceled and just a few others that just showed up. I'll gather them now and update this thread.
I'll still fill out a breaking change form and attend the meeting on Monday.

2022-09-01-preview didn't need breaking change approval. The check passed and in it, it just noted the new field in the new api version and that a ref to a string was replaced with a string which fixed an s360 where x-ms-secret wasn't understood when applied to the ref, but did work directly on the string type.
⚠ 1017 - ReferenceRedirection | The '$ref' property points to different models in the old and new versions.New: Microsoft.StorageCache/preview/2022-09-01-preview/storagecache.json#L3138:9Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L3132:9 💬 1041 - AddedPropertyInResponse | The new version has a new property 'scalingFactor' in response that was not found in the old version.New: Microsoft.StorageCache/preview/2022-09-01-preview/storagecache.json#L1897:11Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L1897:11

The 2023-01-01 GA version just received breaking change approval 2 weeks ago.
It added

❌ 1007 - RemovedClientParameter
The new version is missing a client parameter that was found in the old version. Was 'ResourceGroupParameter' removed or renamed?New: Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json#L3481:3Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L3250:3

❌ 1011 - AddingResponseCode
The new version adds a response code '202'.New: Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json#L547:11

❌ 1019 - RemovedEnumValue
The new version is removing enum value(s) 'Cancelled' from the old version.New: Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json#L2119:13Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L1920:13

❌ 1019 - RemovedEnumValue
The new version is removing enum value(s) 'Cancelled' from the old version.New: Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json#L2835:9Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L2624:9

❌ 1025 - RequiredStatusChange
The 'required' status changed from the old version('True') to the new version('False').New: Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json#L2387:11Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L2188:11

❌ 1025 - RequiredStatusChange
The 'required' status changed from the old version('False') to the new version('True').New: Microsoft.StorageCache/preview/2023-03-01-preview/storagecache.json#L463:11Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L461:11

There are many breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/19724/checks?check_run_id=10940083018

@JeffreyRichter Yes, These are from the unmerged GA version 2023-01-01 and the private preview 2022-09-01. Only a few are from some existing issues found during ARM review.
Here is an early comment I left with the details and the PRs with the breaking changes approvals.
For breaking changes, the list in the checks for this api are inherited from the changes that are merged into this api version. https://github.com/Azure/azure-rest-api-specs/pull/22135/checks?check_run_id=10704482577 - 2023-01-01 GA https://github.com/Azure/azure-rest-api-specs-pr/pull/7982/checks?check_run_id=9480440998 - 2022-09-01 private preview
I'm going through the new comments now and doing what I can to work through the comments without additional breaking changes. The only one that should show up is the change from Cancelled to Canceled to match the ARM spec for provisioningState in stroagecache.json. It wasn't part of the codes enum so it can never be returned. For the new amlfilesystem.json I remove the value from the enum.
I'll go through and point out the new ones which I know are Cancelled to Canceled and just a few others that just showed up. I'll gather them now and update this thread.
I'll still fill out a breaking change form and attend the meeting on Monday.

2022-09-01-preview didn't need breaking change approval. The check passed and in it, it just noted the new field in the new api version and that a ref to a string was replaced with a string which fixed an s360 where x-ms-secret wasn't understood when applied to the ref, but did work directly on the string type.
⚠ 1017 - ReferenceRedirection | The '$ref' property points to different models in the old and new versions.New: Microsoft.StorageCache/preview/2022-09-01-preview/storagecache.json#L3138:9Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L3132:9 💬 1041 - AddedPropertyInResponse | The new version has a new property 'scalingFactor' in response that was not found in the old version.New: Microsoft.StorageCache/preview/2022-09-01-preview/storagecache.json#L1897:11Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L1897:11

There are many breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/19724/checks?check_run_id=10940083018

@JeffreyRichter Yes, These are from the unmerged GA version 2023-01-01 and the private preview 2022-09-01. Only a few are from some existing issues found during ARM review.
Here is an early comment I left with the details and the PRs with the breaking changes approvals.
For breaking changes, the list in the checks for this api are inherited from the changes that are merged into this api version. https://github.com/Azure/azure-rest-api-specs/pull/22135/checks?check_run_id=10704482577 - 2023-01-01 GA https://github.com/Azure/azure-rest-api-specs-pr/pull/7982/checks?check_run_id=9480440998 - 2022-09-01 private preview
I'm going through the new comments now and doing what I can to work through the comments without additional breaking changes. The only one that should show up is the change from Cancelled to Canceled to match the ARM spec for provisioningState in stroagecache.json. It wasn't part of the codes enum so it can never be returned. For the new amlfilesystem.json I remove the value from the enum.
I'll go through and point out the new ones which I know are Cancelled to Canceled and just a few others that just showed up. I'll gather them now and update this thread.
I'll still fill out a breaking change form and attend the meeting on Monday.

2022-09-01-preview didn't need breaking change approval. The check passed and in it, it just noted the new field in the new api version and that a ref to a string was replaced with a string which fixed an s360 where x-ms-secret wasn't understood when applied to the ref, but did work directly on the string type.
⚠ 1017 - ReferenceRedirection | The '$ref' property points to different models in the old and new versions.New: Microsoft.StorageCache/preview/2022-09-01-preview/storagecache.json#L3138:9Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L3132:9 💬 1041 - AddedPropertyInResponse | The new version has a new property 'scalingFactor' in response that was not found in the old version.New: Microsoft.StorageCache/preview/2022-09-01-preview/storagecache.json#L1897:11Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L1897:11

The 2023-01-01 GA version had breaking changes approval 2 weeks ago and added

❌ 1011 - AddingResponseCode
The new version adds a response code '202'.New: Microsoft.StorageCache/stable/2023-01-01/storagecache.json#L555:11

❌ 1025 - RequiredStatusChange
The 'required' status changed from the old version('False') to the new version('True').New: Microsoft.StorageCache/stable/2023-01-01/storagecache.json#L471:11Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L461:11

❌ 1025 - RequiredStatusChange
The 'required' status changed from the old version('False') to the new version('True').New: Microsoft.StorageCache/stable/2023-01-01/storagecache.json#L1089:11Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L1001:11

❌ 1044 - XmsLongRunningOperationChanged
The new version has a different 'x-ms-longrunning-operation' value than the previous one.New: Microsoft.StorageCache/stable/2023-01-01/storagecache.json#L519:7Old: Microsoft.StorageCache/stable/2022-05-01/storagecache.json#L508:7

@JeffreyRichter
The new breaking changes are
1007 - RemovedClientParameter | The new version is missing a client parameter that was found in the old version. Was 'ResourceGroupParameter' removed or renamed?
The new version is missing a client parameter that was found in the old version. Was 'ResourceGroupParameter' removed or renamed?
This was due to a comment during review to use common-types instead of older types we originally had with the same content for the parameter.
1019 - RemovedEnumValue | The new version is removing enum value(s) 'Cancelled' from the old version.
There are 2 cases of this.  The comment was that we used Cancelled instead of Canceled which didn't meet the ARM contract.
I looked at our code and found that value doens't exist in the enum we use and can never come out.  I updated the swagger to match the ARM contract but did not remove the value.
1025 - RequiredStatusChange	The 'required' status changed from the old version('True') to the new version('False').
One instance of this was added after the reviewer pointed out that this field was modifiable and that it wouldn't necessarily need to be supplied on every update.  That matches what the code has been doing so it was updated to point out that the field is not required.
1027 - DefaultValueChanged	The new version has a different default value than the previous one.
There are 3 instances of this.  The reviewer pointed out an issue with the examples for these 3 fields.
They were bools that seemed to imply a default value but one wasn't listed.
The code does default these to false, so I updated the default for these 3.


Regression in complex_layout_semantics_perfinitialSemanticsTreeCreation

Noisy; may have started with engine roll https://github.com/flutter/flutter/commit/37521710aa2f3af0f105bafcb27d7e49c40278e3.

Results for roughly 10/18 - 10/23

This benchmark is all over the place, I'm not sure I'd say it was a regression and I'm not sure what the best thing to do about it is.

Doesn't look like a regression. Closing to unblock the roll.


Update README.md

This is a small cahnge to test a PR
Change
{Describe what kind of change does this PR introduce}
Does this PR introduce a breaking change?
{...}
What needs to be documented once your changes are merged?
{...}
Additional Comments
{...}

This is a comment on the PR


Refactor aggregation-functions to avoid data

Please confirm that this PR has done the following:

~Tests Added~ -> no functional change
~Documentation Added~ -> no functional change
~Name of contributors Added to AUTHORS.rst~
[x] Description in RELEASE_NOTES.md Added

Description of PR
This PR removes two cases where the full data dataframe was created unnecessarily as part of the region-aggregation, refactoring to work directly on the pd.Series.index and related functions. Should speed up some computations a little...
It also harmonizes the log and error messages to be more like pandas - start with a capital letter and use f-strings.

recursive_aggregate as well as aggregate_region still use the data attribute. Is that intentional?

Now that you mention it... Replaced it is in a number of places. But recursive_aggregate() is a bit tricky, not sure how good the unit tests for that function are (found something that should have broken it but didn't). So I'm a bit hesitant to touch that as part of this PR.

The PR was extended to also refactor the recursive-aggregation feature.
Per a bilateral comment by @tburandt, I also harmonized the return type of the aggregate() and aggregate_region() methods when performing "empty" aggregation, i.e., no components exist to perform the requested aggregation.
In the new implementation, an empty IamDataFrame is returned if append=False (instead of None).

Thanks @coroa for the helpful review!


Control of Ventilation-Device

HI,
I really like the idea, but is there any plan to offer control over mqtt?
https://github.com/michaelarnauts/aiocomfoconnect has implemented functions to control the device, so I think in combination with a mqtt-subscription it would be a remarkable extension of this project.
BR,
Simon

Hi,
as you point out, device control is an obvious extension, as it's already in the underlying aiocomfoconnect library. Personally, I haven't implemented any control use cases yet.
Do you have any specific use cases in mind, giving an idea what to implement first? Are you interested in using such an extension, and contribute with testing and/or development?

In my case, the most helpful control would be ventilation speed and switching between manual and auto-mode.
Further controls like party timer, away mode, bypass On-Off, temperture profile and thresholds, etc. are nice to have, but not the most important ones (for me).
Of course I'm interested in using those extensions and I of course could test them.
If wanted I also could participate in development, but I've got quite limited spare time (I guess like anyone ;) )

Sounds sensible. I've made a first shot at this in #7. You can try it out if you like to. It should be mostly complete. What's not added is the ability to set timeouts for bypass, balancemode, boost, away, comfocoolmode, and temperatureprofile.

Thank you for the quick implementation. For your information: I'm using it as docker-image.
A short test of setting and getting mode and speed works great, but only after modifying the reporting-interval parameters.
It seems that they are interpreted as strings, so I had to explicitly cast them as float-values (I locally did this in the init-method of the Reporting-class.
I guess there's a better way than my quick and dirty hack, but for the moment it works for me ;)

Oh, the numbers were not cast properly. Is done now. I've casted the seconds as int and the other number as float.
I've merged everything into master.

@silentsimmal FYI, I've made a release with a prebuilt docker image: https://hub.docker.com/r/ogisnas/comfoconnect-mqtt-bridge

Ok, good to know.
I planning to extend the methods to set the timeouts manually instead of using the default values of the underlying aiocomfoconnect lib, but as already mentioned my spare time is limited, so it'll take some time.
I'll let you know when I've implemented this.


Server Crash

Minecraft v1.15.2
Forge v31.2.45
Damage Tilt v0.1.0
At the time of the crash, I had not taken any damage, but I did have a mob farm running. A zombie was taking damage from a Player Damage Trap from the mod Dark Utilities when the server crash occured. From taking a glance at the log, a zombie may have had an armor piece on with the Steadfast Enchantment from the FF's Enchantment mod.
Crash Report

I see, I don't have any failsafe for fake players getting knocked back... oops
Player Trap attacks zombie
Zombie has Steadfast
Zombie gives knockback to the Player Trap's fake player
Fake player has no "connection" to the server
Crash!
I will try to fix this as soon as I can, thank you for the report!

This has been fixed and released with v0.1.1.
Thanks again!

Thanks!


Disable flags from custom commands when the annotation is not present

Disables flag parsing for custom commands and some internal ones.
Attempts to solve #4225.

Apologize for the quick PR, will actually try to work on this better later, probably locally

@rfay this is kind of working, for now seeing if you like the idea.. Is there any other non-custom command like composer where this should be applied to?

Maybe we can this optional by setting Flags to false or some other annotation?

Related:

https://github.com/drud/ddev/issues/3743

@gilbertsoft ok, new approach, by default it is as it's always have been, but you can now disable them with
## Flags: false

annotation.
This will make the command have full ownership of whatever argument you pass to it.

As per discord, instead of disabling by annotation, simply disables flag parsing if no Flags annotation is present.

Tested and looks really good so far. One thing, we shouldn't change the composer create and create-prjoect because there is the -y option which is used in many places and will lead to errors.

@gilbertsoft thanks, I will think a bit more on the composer. I think ddev composer is actually OK to be stripped of flag parsing, create-project could be an exception.
Also, I had to revert one of your suggestions:
https://github.com/drud/ddev/pull/4283/commits/dc7452243746e0a40b6e3a7a4464c447844f9745
The ok variable is not present outside of the if.

I refactored the composer part a bit, this is my current take:
* custom commands without flags have flag parsing disable

* `ddev composer` has flag parsing disable

* `ddev composer create-project` has flag parsing disable (which will give the unsupported error no matter what you pass on, I think this is ok) and I also added a short description to the command.

Makes sense, yes!

* `ddev composer create` works exactly as before.

I also removed the help function override I was doing to hide --json-output argument from ddev help composer and ddev help composer create. I have a feeling that this command should not be made global as it really only applies to ddev internals (unless I am mistaken) but I rather tackle that on another issue, and my approach was causing a bunch of infinite recursion.
I think the balance of functionality and things that one would expect, like running ddev composer --help to get the composer command help from container and ddev composer --version to return the composer version works nicely. ddev composer create being an additional ddev added wrapper of functionality was left as it is. I do think that we are missing what you would get with composer create-project --help, that needs a bit of thinking.

Yeah, have to check this and think about....

I am considering adding a secondary --help-create-project or seeing if we can properly override the help function to display both the wrapper cobra help as well as the composer create-project output or maybe instruct the user to run the secondary help command. I like this approach best otherwise the output will be too long. We can also tackle on different issues.
I will add both to discuss them independently

It looks like the TestCustomCommands shouldn't be too hard to sort out. Thanks for the great effort on this. It's really important.

how odd that previous tests didn't failed, or at least I think they didn't

Hoping that you'll be able to carry this forward! Thanks for all the good work here.

Whats missing ? Tests ?
 Thanks for all the good
work here.
—
Reply to this email directly, view it on GitHub
https://github.com/drud/ddev/pull/4283#issuecomment-1289770510, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAFFP55E6FOSL44GQYQ54JDWE4KXBANCNFSM6AAAAAARCMM6YY
.
You are receiving this because you modified the open/close state.Message
ID: @.***>

--
Reply might shorter than usual / Respuesta más resumida qué de costumbre
Enviado desde el celular / Sent from Mobile

@gilbertsoft 's review requests? Are they resolved?

Rebased.

@gilbertsoft 's review requests? Are they resolved?

Yes, they are either resolved or addressed.

OK, I re-requested review from him.

Looks like there are some test problems, especially TestCustomCommands.

Rebased, fixed TestCustomCommands, added flags annotation to provided custom commands that needed it.

@hanoii @gilbertsoft hope you can take a look at this, verify it's ready to go in.