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

Add war deployment check #1356

Merged
merged 2 commits into from
Mar 24, 2023
Merged

Add war deployment check #1356

merged 2 commits into from
Mar 24, 2023

Conversation

MichaelsJP
Copy link
Member

@MichaelsJP MichaelsJP commented Mar 22, 2023

Currently, the war deployment is not checked. This starts the container and waits for the health API to indicate the default graph is built and ready.

Pull Request Checklist

  • 1. I have [rebased][rebase] the latest version of the master branch into my feature branch and all conflicts
    have been resolved.
  • 4. I have removed unnecessary commented out code, imports and System.out.println statements.
  • 8. I have built graphs with my code of the Heidelberg.osm.gz file and run the api-tests with all test passing
  • 9. I have referenced the Issue Number in the Pull Request (if the changes were from an issue).
  • 12. I have written in the Pull Request information about the changes made including their intended usage
    and why the change was needed.

Closes #1355

Information about the changes

  • Key functionality added: Test for correct war deployment.

@MichaelsJP MichaelsJP linked an issue Mar 22, 2023 that may be closed by this pull request
@takb takb added this to To do in ors general Mar 22, 2023
Currently, the war deployment is not checked. This starts the container and waits for the health API to indicate the default graph is built and ready.
@MichaelsJP MichaelsJP marked this pull request as ready for review March 22, 2023 14:42
@MichaelsJP MichaelsJP requested review from jlink and koebi March 22, 2023 14:42
Copy link
Contributor

@jlink jlink left a comment

Choose a reason for hiding this comment

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

Does it work - asking because the build step seems to be hanging.

I don't understand all the details of the new build step, what strikes me is that the test will only be run on CI. That's rather late feedback. If that's ok, depends on how often we expect a problem with the war deployment.

@MichaelsJP
Copy link
Member Author

MichaelsJP commented Mar 23, 2023

@jlink

Does it work - asking because the build step seems to be hanging.

Yes. The action name has changed, and it's still looking for the old. Will change that before the merge. Else, all other PRs will have to wait for it.

I don't understand all the details of the new build step, what strikes me is that the test will only be run on CI. That's rather late feedback. If that's ok, depends on how often we expect a problem with the war deployment.

I'm open to ideas on how to solve that deployment scenario locally, as isolated as it is in the workflow with the Dockerfile. In my opinion, it's ok to have that kind of feedback latest with a PR, so there is no danger of merging it into master. It also runs on a draft PR, so feedback is given quick if commits are uploaded regularly.

@MichaelsJP MichaelsJP changed the title build(github): Add war deployment check Add war deployment check Mar 23, 2023
@MichaelsJP MichaelsJP self-assigned this Mar 23, 2023
@MichaelsJP MichaelsJP requested a review from jlink March 24, 2023 00:51
@MichaelsJP MichaelsJP linked an issue Mar 24, 2023 that may be closed by this pull request
Copy link
Contributor

@jlink jlink left a comment

Choose a reason for hiding this comment

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

Let's start with your approach and think about a way to extract that from GH actions later.

@MichaelsJP MichaelsJP merged commit db2f76f into master Mar 24, 2023
ors general automation moved this from To do to Awaiting release Mar 24, 2023
@MichaelsJP MichaelsJP deleted the 1355-add-war-deployment-test branch March 24, 2023 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
ors general
  
Awaiting release
Development

Successfully merging this pull request may close these issues.

Add war deployment test test war-file deployment
2 participants