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

verify docker image #274

Merged
merged 2 commits into from
Sep 19, 2024
Merged

verify docker image #274

merged 2 commits into from
Sep 19, 2024

Conversation

salamonpavel
Copy link
Collaborator

@salamonpavel salamonpavel commented Sep 18, 2024

Dockerfile and application configuration verified for upcoming deployment.
Closes #260

Release notes:

  • Dockerfile and application configuration verified for deployment with ZIO and Http4s web server

@salamonpavel
Copy link
Collaborator Author

Release notes:

  • Dockerfile and application configuration verified for deployment with ZIO and Http4s web server

@salamonpavel salamonpavel marked this pull request as ready for review September 18, 2024 10:08
Copy link

JaCoCo model module code coverage report - scala 2.13.11

Overall Project 63.64% 🍏

There is no coverage information present for the Files changed

Copy link

JaCoCo agent module code coverage report - scala 2.13.11

Overall Project 84.63% 🍏

There is no coverage information present for the Files changed

Copy link

JaCoCo reader module code coverage report - scala 2.13.11

Overall Project 100% 🍏

There is no coverage information present for the Files changed

Copy link

JaCoCo server module code coverage report - scala 2.13.11

Overall Project 76.37% 🍏

There is no coverage information present for the Files changed

@@ -55,7 +55,7 @@ object Setup {
}

val serverMergeStrategy = assembly / assemblyMergeStrategy := {
case PathList("META-INF", "services") => MergeStrategy.filterDistinctLines
case PathList("META-INF", "services", _*) => MergeStrategy.concat
Copy link
Contributor

Choose a reason for hiding this comment

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

What if there are same keys? (not sure what strategies there are)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the context of the sbt-assembly plugin, merge strategies determine how to handle files with the same path when creating an assembly JAR. Here are the common merge strategies:

MergeStrategy.concat

  • Purpose: Concatenates the contents of all files with the same path.
  • Usage: Useful when you want to combine the contents of multiple files into one, preserving all lines from each file.
  • Example:
    case PathList("META-INF", "services", _*) => MergeStrategy.concat

MergeStrategy.singleOrError

  • Purpose: Ensures that only one file is included in the final assembly. If multiple files with the same path are found, it throws an error.
  • Usage: Useful when you need to guarantee that there is no ambiguity or conflict for a particular file.
  • Example:
    case PathList("META-INF", "maven", "org.webjars", "swagger-ui", "pom.properties") => MergeStrategy.singleOrError

MergeStrategy.rename

  • Purpose: Handles file conflicts by renaming the conflicting files.
  • Usage: Useful when you want to include all versions of a file without merging their contents.
  • Example:
    case PathList("META-INF", "services", _*) => MergeStrategy.rename

MergeStrategy.discard

  • Purpose: Discards all files with the same path.
  • Usage: Useful when you want to exclude certain files from the final assembly.
  • Example:
    case PathList("META-INF", _*) => MergeStrategy.discard

MergeStrategy.first

  • Purpose: Keeps the first file encountered and discards the rest.
  • Usage: Useful when the order of files is known and you only need the first one.
  • Example:
    case _ => MergeStrategy.first

MergeStrategy.last

  • Purpose: Keeps the last file encountered and discards the rest.
  • Usage: Useful when the order of files is known and you only need the last one.
  • Example:
    case _ => MergeStrategy.last

MergeStrategy.filterDistinctLines

  • Purpose: Merges files by including only distinct lines.
  • Usage: Useful when you want to avoid duplicate lines in the merged file.
  • Example:
    case "application.conf" => MergeStrategy.filterDistinctLines

MergeStrategy.deduplicate

  • Purpose: Merges files by including only distinct files.
  • Usage: Useful when you want to avoid duplicate files in the merged file.
  • Example:
    case "reference.conf" => MergeStrategy.deduplicate

Custom Merge Strategy

  • Purpose: Allows you to define custom logic for merging files.
  • Usage: Useful when none of the predefined strategies fit your needs.
  • Example:
    case "custom.conf" => new MergeStrategy {
      override def name: String = "customMergeStrategy"
      override def apply(tmp: File, path: String, files: Seq[File]): Either[String, Seq[File]] = {
        // Custom merge logic here
        Right(files)
      }
    }

These strategies help manage how files are handled during the assembly process, ensuring that the final JAR is built according to your project's requirements.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have tested that it works also with filterDistinctLines. So let me know which one you prefer. @lsulak @benedeki

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MergeStrategy.filterDistinctLines and MergeStrategy.concat serve different purposes, and the choice between them depends on your specific requirements.

MergeStrategy.filterDistinctLines

  • Purpose: Merges files by including only distinct lines.
  • Usage: Useful when you want to avoid duplicate lines in the merged file.
  • Safety: Safer in scenarios where duplicate lines could cause issues, as it ensures each line appears only once.

MergeStrategy.concat

  • Purpose: Concatenates the contents of all files with the same path.
  • Usage: Useful when you want to combine the contents of multiple files into one, preserving all lines from each file.
  • Safety: May lead to conflicts or unexpected behavior if there are lines with the same keys but different values.

Conclusion

  • filterDistinctLines is generally safer when you need to ensure that there are no duplicate lines in the merged file.
  • concat is useful when you need to preserve all lines, but it requires careful handling of potential conflicts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you very much for the educative window!

I can't tell you my preference because I don't know what duplicates we have here, and what lines. So I will base my answer according to these details - were you able to find out the specifics?

Also, was there a problem so that you changed it, or you just experimented with it? At first, I thought that filterDistinctLines was not working for us, that's why you changed it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The change wasn't just about the strategy. Honestly I didn't dig very deeply into this, just made it work by trial/error. At this moment I would rather dedicate my time to something more pressing than exploring this further.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Understood, well then I don't really have preferences. Whatever works would be my answer, and if we encounter a problem around these, we can revisit it later with this knowledge ^ in mind 🤷

@@ -5,9 +5,9 @@
<Pattern>%d{HH:mm:ss.SSS} %-5level [%thread] %logger{36}:%L - %msg %ex{short}%n</Pattern>
</layout>
</appender>
<root level="debug">
<root level="info">
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

server/src/main/resources/reference.conf Show resolved Hide resolved
keyStorePassword=password
keyStorePath="/path/to/your/cert"
keyStorePassword=changeit
keyStorePath="/etc/ssl/certs/selfsigned.jks"
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps a small comment for this value can be good to have here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lsulak do you have any suggestion for a good comment here?

Copy link
Collaborator

@lsulak lsulak Sep 19, 2024

Choose a reason for hiding this comment

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

not sure, but perhaps something like this?

location of Java Keystore (JKS) file with certificate to be used for HTTPS communication; it can be sufficient to use self-signed certificate on testing or development environment or when application runs in an internal network; otherwise certificates are usually obtained from a trusted CA

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's a bit wordy but thanks will use it

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah I know, feel free to change it :D

@benedeki
Copy link
Contributor

Seems OK to me, but don't feel competent in docker to judge the completeness.

Copy link
Collaborator

@lsulak lsulak left a comment

Choose a reason for hiding this comment

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

Looks good, I like that you also added the docker build & run command into the README with all the parameters we talked about - I didn't try it yet on my machine but we did it together on a call. Approved, good job :)

@salamonpavel salamonpavel merged commit 2203397 into master Sep 19, 2024
10 checks passed
@salamonpavel salamonpavel deleted the feature/260-docker branch September 19, 2024 09:20
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.

Changing/verifying Atum docker definition
3 participants