{{ message }}
Repository navigation
README edits - #39
Closed
patty-chow wants to merge 3 commits into
Closed
README edits#39patty-chow wants to merge 3 commits into
patty-chow wants to merge 3 commits into
Conversation
safern
approved these changes
Jan 29, 2025
sivethe
requested changes
Jan 29, 2025
sivethe
left a comment
Contributor
There was a problem hiding this comment.
Please make these changes in our forward-sync repo
Contributor
Author
guanzhousongmicrosoft
added a commit
that referenced
this pull request
Aug 20, 2026
…lease Squashes the OSS-only fixes prepared for v0.116-0 on top of the internal cherry-picks (documentdb-local, packaging and gateway) already synced from the internal repo. * fix(documentdb-local): interlock the data directory across containers. PostgreSQL decides whether postmaster.pid is stale from kill(pid, 0) and the SysV shared-memory segment. Both are PID-namespace-scoped, so inside containers both are wrong in both directions: a recycled PID makes a dead postmaster look alive (#43), and a live postmaster in another container looks dead (#62). Ownership is no longer decided from a PID. It is taken with an flock(2) claim on the data directory inode, which crosses namespaces and which the kernel releases when the holder dies, so a leftover postmaster.pid is residue by construction. A second container started on a volume already in use now refuses to start and names the conflict, instead of deleting the live lock file and taking both databases down while both containers still report Running=true. Fixes #43. Fixes #62. * fix(emulator): exit on a bare positional instead of spinning at 100% CPU. The argument loop's only fallback arm was `-*)`, so a token that does not start with `-` matched no arm at all. Nothing shifted, `$#` never decreased, and the loop never terminated: the container burned a full CPU core, emitted no diagnostic, never started, and still reported Running=true to Docker. A `*)` catch-all now reports the offending token on stderr and exits 1. Fixes #61. * fix(ci): publish container signatures cosign v2 clients can verify. Pair --new-bundle-format=false with --use-signing-config=false so cosign v3 keeps publishing the legacy .sig tag, and assert that tag is present after signing so a regression fails the build rather than the consumer. * fix(packaging): forward SOURCE_DATE_EPOCH into the build containers. The per-format workflows pin it to the commit timestamp, but only in the runner's environment; the extras are assembled on the host and do see it, while the extension and gateway packages are built inside Docker and the four package-producing `docker run` calls did not forward it. Unset in the container, deb_install_changelog falls back to `date -u` and stamps each cell's own HH:MM:SS into the shipped /usr/share/doc/<pkg>/changelog.gz. The gateway package is PG-agnostic, so the pg17 and pg18 cells emit the same filename and build_all_packages.yml requires those copies to be structurally identical, comparing every payload entry's size; gzip -9n of that changelog is 172-174 bytes depending only on the timestamp digits, so two cells agree only about half the time and the release bundle fails intermittently. Forwarded with the valueless `--env SOURCE_DATE_EPOCH` form, which passes the host value through when set and leaves it unset otherwise, so a local build behaves exactly as it does today and no illegal empty value is ever injected. * docs(readme): persist data with a volume and document readiness in the quickstart. The quickstart's docker run mounted no volume, so data was silently lost on container removal, and readiness was never mentioned. Also corrects the credential and reset semantics. Fixes #39. * Set the v0.116-0 release date and mark v0.115-0 as skipped. The v0.115-0 release was skipped; its changes are included in v0.116-0. Verified end to end against the 0.116.0 images on PG 15, 16, 17 and 18: a bare positional exits 1 with a diagnostic at 0% CPU; a second container on a live volume is refused while the first keeps serving with its postmaster.pid untouched and 2000 documents intact; 10 of 10 unclean restart cycles boot, clear the residue and preserve the data. Entrypoint unit suite: 134 tests pass. cosign v2.4.1 verifies the published manifests. Driving deb_install_changelog as two matrix cells one second apart: 6 of 12 pairs disagreed in size before the pin, 0 of 12 after. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Guanzhou Song <guanzhousong@microsoft.com>
guanzhousongmicrosoft
added a commit
that referenced
this pull request
Aug 21, 2026
…lease Squashes the OSS-only fixes prepared for v0.116-0 on top of the internal cherry-picks (documentdb-local, packaging and gateway) already synced from the internal repo. * fix(documentdb-local): interlock the data directory across containers. PostgreSQL decides whether postmaster.pid is stale from kill(pid, 0) and the SysV shared-memory segment. Both are PID-namespace-scoped, so inside containers both are wrong in both directions: a recycled PID makes a dead postmaster look alive (#43), and a live postmaster in another container looks dead (#62). Ownership is no longer decided from a PID. It is taken with an flock(2) claim on the data directory inode, which crosses namespaces and which the kernel releases when the holder dies, so a leftover postmaster.pid is residue by construction. A second container started on a volume already in use now refuses to start and names the conflict, instead of deleting the live lock file and taking both databases down while both containers still report Running=true. Fixes #43. Fixes #62. * fix(emulator): exit on a bare positional instead of spinning at 100% CPU. The argument loop's only fallback arm was `-*)`, so a token that does not start with `-` matched no arm at all. Nothing shifted, `$#` never decreased, and the loop never terminated: the container burned a full CPU core, emitted no diagnostic, never started, and still reported Running=true to Docker. A `*)` catch-all now reports the offending token on stderr and exits 1. Fixes #61. * fix(ci): publish container signatures cosign v2 clients can verify. Pair --new-bundle-format=false with --use-signing-config=false so cosign v3 keeps publishing the legacy .sig tag, and assert that tag is present after signing so a regression fails the build rather than the consumer. * fix(packaging): forward SOURCE_DATE_EPOCH into the build containers. The per-format workflows pin it to the commit timestamp, but only in the runner's environment; the extras are assembled on the host and do see it, while the extension and gateway packages are built inside Docker and the four package-producing `docker run` calls did not forward it. Unset in the container, deb_install_changelog falls back to `date -u` and stamps each cell's own HH:MM:SS into the shipped /usr/share/doc/<pkg>/changelog.gz. The gateway package is PG-agnostic, so the pg17 and pg18 cells emit the same filename and build_all_packages.yml requires those copies to be structurally identical, comparing every payload entry's size; gzip -9n of that changelog is 172-174 bytes depending only on the timestamp digits, so two cells agree only about half the time and the release bundle fails intermittently. Forwarded with the valueless `--env SOURCE_DATE_EPOCH` form, which passes the host value through when set and leaves it unset otherwise, so a local build behaves exactly as it does today and no illegal empty value is ever injected. * docs(readme): persist data with a volume and document readiness in the quickstart. The quickstart's docker run mounted no volume, so data was silently lost on container removal, and readiness was never mentioned. Also corrects the credential and reset semantics. Fixes #39. * Set the v0.116-0 release date and mark v0.115-0 as skipped. The v0.115-0 release was skipped; its changes are included in v0.116-0. Verified end to end against the 0.116.0 images on PG 15, 16, 17 and 18: a bare positional exits 1 with a diagnostic at 0% CPU; a second container on a live volume is refused while the first keeps serving with its postmaster.pid untouched and 2000 documents intact; 10 of 10 unclean restart cycles boot, clear the residue and preserve the data. Entrypoint unit suite: 134 tests pass. cosign v2.4.1 verifies the published manifests. Driving deb_install_changelog as two matrix cells one second apart: 6 of 12 pairs disagreed in size before the pin, 0 of 12 after. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Guanzhou Song <guanzhousong@microsoft.com>
guanzhousongmicrosoft
added a commit
that referenced
this pull request
Aug 22, 2026
…lease Squashes the OSS-only fixes prepared for v0.116-0 on top of the internal cherry-picks (documentdb-local, packaging and gateway) already synced from the internal repo. * fix(documentdb-local): interlock the data directory across containers. PostgreSQL decides whether postmaster.pid is stale from kill(pid, 0) and the SysV shared-memory segment. Both are PID-namespace-scoped, so inside containers both are wrong in both directions: a recycled PID makes a dead postmaster look alive (#43), and a live postmaster in another container looks dead (#62). Ownership is no longer decided from a PID. It is taken with an flock(2) claim on the data directory inode, which crosses namespaces and which the kernel releases when the holder dies, so a leftover postmaster.pid is residue by construction. A second container started on a volume already in use now refuses to start and names the conflict, instead of deleting the live lock file and taking both databases down while both containers still report Running=true. Fixes #43. Fixes #62. * fix(emulator): exit on a bare positional instead of spinning at 100% CPU. The argument loop's only fallback arm was `-*)`, so a token that does not start with `-` matched no arm at all. Nothing shifted, `$#` never decreased, and the loop never terminated: the container burned a full CPU core, emitted no diagnostic, never started, and still reported Running=true to Docker. A `*)` catch-all now reports the offending token on stderr and exits 1. Fixes #61. * fix(ci): publish container signatures cosign v2 clients can verify. Pair --new-bundle-format=false with --use-signing-config=false so cosign v3 keeps publishing the legacy .sig tag, and assert that tag is present after signing so a regression fails the build rather than the consumer. * fix(packaging): forward SOURCE_DATE_EPOCH into the build containers. The per-format workflows pin it to the commit timestamp, but only in the runner's environment; the extras are assembled on the host and do see it, while the extension and gateway packages are built inside Docker and the four package-producing `docker run` calls did not forward it. Unset in the container, deb_install_changelog falls back to `date -u` and stamps each cell's own HH:MM:SS into the shipped /usr/share/doc/<pkg>/changelog.gz. The gateway package is PG-agnostic, so the pg17 and pg18 cells emit the same filename and build_all_packages.yml requires those copies to be structurally identical, comparing every payload entry's size; gzip -9n of that changelog is 172-174 bytes depending only on the timestamp digits, so two cells agree only about half the time and the release bundle fails intermittently. Forwarded with the valueless `--env SOURCE_DATE_EPOCH` form, which passes the host value through when set and leaves it unset otherwise, so a local build behaves exactly as it does today and no illegal empty value is ever injected. * docs(readme): persist data with a volume and document readiness in the quickstart. The quickstart's docker run mounted no volume, so data was silently lost on container removal, and readiness was never mentioned. Also corrects the credential and reset semantics. Fixes #39. * Set the v0.116-0 release date and mark v0.115-0 as skipped. The v0.115-0 release was skipped; its changes are included in v0.116-0. Verified end to end against the 0.116.0 images on PG 15, 16, 17 and 18: a bare positional exits 1 with a diagnostic at 0% CPU; a second container on a live volume is refused while the first keeps serving with its postmaster.pid untouched and 2000 documents intact; 10 of 10 unclean restart cycles boot, clear the residue and preserve the data. Entrypoint unit suite: 134 tests pass. cosign v2.4.1 verifies the published manifests. Driving deb_install_changelog as two matrix cells one second apart: 6 of 12 pairs disagreed in size before the pin, 0 of 12 after. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Guanzhou Song <guanzhousong@microsoft.com> --- Add RHEL prerequisite validation and extraction scripts for DocumentDB packaging - Introduced `extract-el-prereqs.sh` to extract and validate RHEL prerequisite commands from README.md. - Added `test-documented-prereqs.sh` to verify that documented prerequisites are sufficient for RPM installation on stock EL hosts. - Updated workflows to utilize the new scripts for generating and checking prerequisites, ensuring consistency and preventing drift. --- Fix review findings in the EL prerequisite tooling (GitHub issue #75) Review findings folded in rather than left as a follow-up: User-facing correctness: - Document the real-RHEL path. `dnf config-manager --set-enabled crb` fails on subscribed RHEL, which has no `crb` repo id; the block now also carries the `subscription-manager repos --enable codeready-builder-...` form. Comment lines are inert when the block is executed, so it stays runnable verbatim. Stop a job summary from gating the release: - build_all_packages.yml's summary step is presentation-only and is the LAST step of `aggregate`, a `needs:` of create-release, running after the bundle is built and uploaded. It now warns and omits instead of exit 1. documentdb_release.yml still hard-fails, so the release path stays gated. Make the suite stop lying about why something failed: - A failing prerequisite command cascaded into a depsolve error, which the classifier refuses to retry and reported as "the docs are insufficient". The runner now fails fast and reports the prerequisite as the cause. - The negative control matched libqhull_r BEFORE checking for a depsolve signature, so a download-phase failure whose transaction table merely lists libqhull_r was certified as "issue #75 reproduces". Reordered. Guards that did not guard: - One tripwire per documented command, not two. A deletion was invisible: --check-copies stays green (other copies remain a superset) and the install test does not backstop `module disable postgresql`. - --check-copies scoped to %description. It grepped the whole spec, so moving the guidance to RPM comments (never shipped) still reported OK. - Assert exactly one BEGIN/END marker pair; a stray marker silently concatenated unrelated README lines into the executed command list. - Fail closed when the EL/arch rewrite does not match, instead of emitting an internally contradictory block. - Normalise EL major and arch when comparing copies, so an arch-correct %description is not reported as drift. Portability and hygiene: - Anchor the PREREQ_EXIT/INSTALL_EXIT greps at column 0: `set -x` echoes the trap definition, whose text contains the marker, so every run reported a prerequisite failure that never happened. - Use `sed -E`; BSD sed treats `\+` as a literal plus and skipped the rewrite. - Absolutise the RPM path before docker sees it (a relative --packages-dir was read as a named volume). - Exclude debuginfo/debugsource siblings from the RPM glob. - Scope the drift gate to one matrix cell and note that this workflow's push trigger path-ignores '**/*.md'. Verified on stock images against the real spec Requires set: EL9/x86_64 PG17 and PG18, EL9/aarch64 PG18, EL8/x86_64 PG17. Each also reproduces the #75 failure when CRB/powertools is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRK9WmRwAerTi9Am5ogbGH --- Create documentdb_extended_rum on the packaged install path documentdb-tune writes documentdb.alternate_index_handler_name='extended_rum' whenever the extended-RUM control file is present on the host, but the access method that GUC names only exists in a database where documentdb_extended_rum has been created. That is per-database catalog state no config file can supply, and CREATE EXTENSION documentdb CASCADE does not pull it in. The result was a tuned cluster where every createIndexes failed with "Index access method extended_rum is not available". The control file ships inside postgresql-N-documentdb itself, so the probe is always true and the GUC is always written. The container and the documentdb-setup wizard were unaffected because both already create the extension; only the documented manual path broke. Adding documentdb_extended_rum to documentdb.control's requires is not a viable fix: the extension errors out of _PG_init unless it is already in shared_preload_libraries, which CASCADE cannot arrange. - Single-source the extension-creation guidance in documentdb-tools-lib.sh so the tools cannot drift on which extensions a tuned cluster needs. - documentdb-tune now prints the exact CREATE EXTENSION command, including documentdb_extended_rum when it wrote the extended-RUM GUCs. - documentdb-createcluster --start creates the extensions, so the documented one-step path yields a cluster that can create indexes. - documentdb-gateway-admin --check and documentdb-register-gateway no longer tell operators to run only CREATE EXTENSION documentdb CASCADE, which is what produced the broken state. - Correct Workflows A and B in packaging-design.md and packaging/README.md. - Add package-test coverage: assert index creation works end to end, and assert documentdb-tune's output names documentdb_extended_rum. The existing suite only exercised the wizard path, which is why this was not caught. Fixes #74 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 729aef82-6b91-41ae-b513-542103e879d3 Signed-off-by: Guanzhou Song <guanzhousong@microsoft.com> --- Keep psql stderr out of the index-probe assertion verify_index_creation_works captured psql output with 2>&1, so the server's "NOTICE: creating collection" was spliced into the value compared against 'ok'. The index was created successfully; only the assertion failed. Silence the notice with client_min_messages, and redirect stderr to a temp file instead of merging it, so the captured value is the ok flag alone while failures still report the backend error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 729aef82-6b91-41ae-b513-542103e879d3 Signed-off-by: Guanzhou Song <guanzhousong@microsoft.com> --- Resolve the extension hint against the cluster the tool is talking about The extension-creation guidance added earlier in this change was correct only on a single-cluster host with a live, restarted server. Three gaps: 1. The printed psql command carried no -h/-p/--cluster, so on a host with more than one cluster -- or the stand-alone documentdb-local instance on port 9700+major -- it silently created the extensions somewhere else and reported success. documentdb_create_extension_command now takes an optional conn_spec and every call site passes the coordinates it already held. documentdb-tune additionally reads the data directory's owner instead of assuming "postgres", which fails peer auth on those instances. 2. The handler probe read documentdb.alternate_index_handler_name and treated an empty result as "no extra extension needed". current_setting() returns NULL for an unknown GUC, and pg_documentdb is not preloaded until PostgreSQL restarts -- exactly the window documentdb-register-gateway warns in -- so the advice reverted to the single-statement recipe when it mattered most. The GUC's own default is "" too, so the two states had to be told apart rather than merged: the read now returns 2 for an absent GUC and falls back to the same on-disk control-file probe documentdb-tune used to decide to pin the handler. 3. documentdb-gateway-admin's new access-method probe used `2>/dev/null || true`, so a permission error or dropped connection printed a confident "Index creation will fail" about a healthy database. It now captures stderr separately and dies on a probe failure, matching the pattern already used in cmd_reset_password. Also: documentdb-tune printed its guidance before the restart instruction on the "config is already up to date" path, and under --dry-run, and claimed the extensions belong in "each database" when the managed block pins cron.database_name='postgres' and documentdb requires pg_cron -- so only the postgres database can work. The restart guidance is extracted so every path prints it first, and a --no-next-steps flag lets documentdb-createcluster suppress tune's copy: with --start it has already created the extensions, and without --start tune's copy landed above "Start with: pg_ctlcluster ...", telling the operator to run CREATE EXTENSION against a stopped cluster. documentdb-createcluster keeps failing open when the share directory cannot be read, now with the reasoning recorded: documentdb-tune's resolve_pg_sharedir swallows the identical failure and therefore does not pin the handler either, so skipping the extension stays consistent with the config that was written. It gains a psql preflight and prints the manual command when --start's extension creation fails, where the cluster is already created and running and the recovery hint on the no-start path is unreachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wf1jNJ5hYsw2KV4NQeqjAQ --- Fix the package banners that still print the single-statement recipe The gateway package banners were missed entirely by the extended-RUM pass above. Both the DEB postinst and the RPM %post scriptlet still printed sudo -u postgres psql -c 'CREATE EXTENSION documentdb CASCADE;' inside their Workflow B recipe -- the highest-traffic copy in the repo, and the one an operator sees on `apt install documentdb-gateway`. Every step of that chain succeeds; the failure surfaces much later on the first createIndexes, with nothing in the install output hinting a second statement was needed. That also falsified the "single-sourced so those hints cannot drift" claim in documentdb-tools-lib.sh: the drift was present at merge. These two copies cannot source documentdb-tools-lib.sh -- it ships in documentdb-postgresql-tools, which need not be installed when the scriptlet runs -- so they stay static duplicates. verify_install_banner_extension_hint in both packaged-install E2E suites now asserts them against the INSTALLED scriptlet (/var/lib/dpkg/info/*.postinst, rpm -q --scripts), which checks the shipped artifact rather than the source tree, and rejects a partial revert to the single-statement form. Coverage for the behaviour this PR advertises: - documentdb-createcluster --start had never been executed anywhere. The only invocation in the repo is e2e-packaging-smoke-test.sh, without --start, in the non-Debian branch that exits early with its status discarded -- and packaging-design.md deleted the manual CREATE EXTENSION fallback in the same PR, so a broken --start would have left the documented path with no escape hatch. verify_createcluster_start_creates_extensions builds a throwaway cluster, asserts both extensions exist in THAT cluster, asserts an index actually builds, and checks the no-start banner is runnable top to bottom (start command first, recipe printed exactly once, naming --cluster N/name). - The index probe selected only `ok`. ok=false is reported in-band -- psql exits 0 with empty stderr and the reason lives in retval -- so the single regression that test exists to catch reported as "got: 'f'", naming neither the access method nor the missing extension. It now selects retval too. packaging/README.md framed `documentdb-createcluster 18 main --start` as a one-step form of Workflow A, but postgresql-18 auto-creates cluster main on install, so pg_createcluster aborts with "cluster 18/main already exists". Restored the "starting from scratch" qualifier that packaging-design.md kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wf1jNJ5hYsw2KV4NQeqjAQ --- Fix ordering, coordinates, and preflight gaps found reviewing the fixes Five defects from a cold review of the two preceding sections, all in the same class the original PR was fixing -- guidance that cannot be followed as printed: - documentdb-register-gateway printed the extended_rum recipe followed only by a reload. That warning fires in the post-tune/pre-restart window (the window the new on-disk fallback exists to serve), where pg_documentdb_extended_rum is not yet in shared_preload_libraries, so the statement errors out of _PG_init -- and a reload never applies shared_preload_libraries. It now states the restart precondition before the recipe, which is a no-op for an operator who already restarted. - --no-next-steps suppressed only the CREATE EXTENSION hint, not print_restart_instruction, so documentdb-createcluster without --start printed "Restart the cluster to apply: systemctl restart postgresql@18-foo" and then "Start with: pg_ctlcluster 18 foo start" -- two different commands for a cluster that has never been started. The flag now means the wrapper owns all post-apply guidance, restart line included. - The gateway banners were the one recipe in this change that gained no cluster coordinates. Every other line is parameterised on <N>, but a bare psql goes to whatever owns the default socket, so on a host where a different major is the default the extensions land there and the documentdb-register-gateway line below reports them missing. The DEB banner now passes --cluster <N>/main; the RPM one keeps a bare psql (--cluster is Debian pg_wrapper only) with the multi-major caveat on a separate advisory line -- not an inline comment, since a '#' inside a backslash-continued command comments out the continuation and truncates the recipe when pasted. verify_install_banner_extension_hint now asserts the coordinates too; it would not have caught this. - documentdb-createcluster's psql preflight was unconditional and ran before argument parsing, so the no-start path -- which never invokes psql -- began dying on hosts without a client package. Moved after parsing and gated on --start. - The new --start probe merged stderr into the captured row and then prefix-matched "t|", so any WARNING ahead of the row would fail the test on a healthy cluster. Now uses the same separate-stderr-file handling as the sibling verify_index_creation_works, which documents that exact reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wf1jNJ5hYsw2KV4NQeqjAQ --- Compact the comments added by the extended-RUM fixes Comment-only pass over the three preceding sections: same facts, roughly a third fewer lines. The reasoning that is not recoverable from the code is kept -- why the fail-open probe is deliberate, why the absent-vs-empty GUC split exists, why the banners are static duplicates, why a '#' cannot go inside the RPM recipe -- while the restatements of what the code plainly does are gone. No functional change: comment lines and blanks stripped, every touched file hashed identically before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wf1jNJ5hYsw2KV4NQeqjAQ --- Quiet psql in the createcluster probe so the result row stands alone Found by running the packaged-install E2E: the new --start probe captured "SET\nSET\nt|BSONHEX..." and failed its `t|` prefix match. Without -q psql prints a command tag for each SET ahead of the result row. run_psql already uses -Atq for exactly this reason; the new probe was the only psql call in the suite that omitted it. The code under test was correct -- the same run reported numIndexesBefore=1, numIndexesAfter=2, ok=1, so documentdb-createcluster --start had created both extensions and the index built. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wf1jNJ5hYsw2KV4NQeqjAQ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

fit doc to PG compliance