Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log error when no compose files were overwritten on upgrade complete #7863

Merged
merged 6 commits into from Nov 8, 2022

Conversation

dianabarsan
Copy link
Member

@dianabarsan dianabarsan commented Oct 24, 2022

Description

#7859

Code review checklist

  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Documented: Configuration and user documentation on cht-docs
  • Tested: Unit and/or e2e where appropriate
  • Internationalised: All user facing text
  • Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

Compose URLs:

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

Signed-off-by: Diana Barsan <barsan@medic.org>
Signed-off-by: Diana Barsan <barsan@medic.org>
Signed-off-by: Diana Barsan <barsan@medic.org>
@dianabarsan
Copy link
Member Author

Penny for your thoughts, please, @garethbowen . Thanks!

Copy link
Member

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! A few suggestions for readability.


if (!upgradeUtils.upgradeResponseSuccess(payload, response)) {
logger.error('No compose files were updated: %o', response);
throw new Error('No compose files were updated');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I presume this shows the generic error message in the admin UI? If so, we should improve this error message to have more information about what went wrong and how to fix it.

api/src/services/setup/upgrade-steps.js Outdated Show resolved Hide resolved
api/src/services/setup/upgrade-steps.js Show resolved Hide resolved
Signed-off-by: Diana Barsan <barsan@medic.org>
Signed-off-by: Diana Barsan <barsan@medic.org>
Copy link
Member

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks!

…e-kubernetes

Signed-off-by: Diana Barsan <barsan@medic.org>
@dianabarsan
Copy link
Member Author

Hey @garethbowen

I pushed a change which will also treat responses from upgrade-service-kubernetes, but that one still requires changing the endpoint output in the service itself. I also changed the message to make it clearer, as you requested above and I failed to notice before.

Copy link
Member

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better now :)

@dianabarsan dianabarsan merged commit 49f5a60 into master Nov 8, 2022
@dianabarsan dianabarsan deleted the 7859-no-doc-error branch November 8, 2022 18:04
@dianabarsan dianabarsan self-assigned this Nov 22, 2022
elvisdorkenoo added a commit that referenced this pull request Nov 23, 2022
* add e2e tests to messages tab breadcrumbs


* fix breatfeeding typo for delivery form in default config (#7890)

#7884

* add GH Action to replace placeholders with staging urls  (#7860)

* add GH Action for staging urls per #7848


* trying to fix path...how I love how you have to push to test a 1 line change...

* Clean up readme and template changes in prep for PR

* 2 spaces > 4 spaces per feedback

* make search tokens more unique per feedback

* test committ to trigger replace action

* revert pinning to this branch for testing

* collapse decleration of updateme object per feedback

* un-collapse decleration of updateme object against feedback

* fix typo in branch nane

* revert pin to branch

* collapse more objects per feedback

* unping from branch

* Update .github/actions/update-staging-url-placeholders/README.md

Co-authored-by: Gareth Bowen <gareth@medic.org>

Co-authored-by: Gareth Bowen <gareth@medic.org>

* Change staging server db

#7879

* apply review recommendations

* minor changes

* Log error when no compose files were overwritten on upgrade complete (#7863)

#7859

* Update deep staging link: builds -> builds_4

* add missing opening quote on couch readme (#7881)


Co-authored-by: Gareth Bowen <gareth@medic.org>
Co-authored-by: Ashley <8253488+mrjones-plip@users.noreply.github.com>
Co-authored-by: Diana Barsan <35681649+dianabarsan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants