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

SnapStart integration enhancement #29637

Merged
merged 1 commit into from Dec 2, 2022
Merged

Conversation

cescoffier
Copy link
Member

  • Write documentation
  • Use SnapStart instead of CRaC to avoid the confusion

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I added a few suggestions and comments.

docs/src/main/asciidoc/amazon-snapstart.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/amazon-snapstart.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/amazon-snapstart.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/amazon-snapstart.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/amazon-snapstart.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/amazon-snapstart.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/amazon-snapstart.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/amazon-snapstart.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/amazon-snapstart.adoc Outdated Show resolved Hide resolved
- Write documentation
- Use SnapStart instead of CRaC to avoid the confusion
Quarkus Documentation automation moved this from To do to Reviewer approved Dec 2, 2022
@geoand
Copy link
Contributor

geoand commented Dec 2, 2022

Very good idea to break Crac and Snapstart up in the code.

@loicmathieu
Copy link
Contributor

SnapStart is only for Amazon Lambda right ?
Maybe it should be moved to the Amazon Lambda extension.

My understanding is that CRaC support was removed now, it's a little shame as it was promising but I understand that such early tech is not yet supported.

@quarkus-bot
Copy link

quarkus-bot bot commented Dec 2, 2022

Failing Jobs - Building 9f57081

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

@gsmet
Copy link
Member

gsmet commented Dec 2, 2022

@loicmathieu this PR doesn't remove any CRaC feature. CracConfig was introduced very recently specifically for SnapStart. If at some point, it makes sense to generalize, we can still tweak things.

@gsmet gsmet merged commit 8620cf4 into quarkusio:main Dec 2, 2022
Quarkus Documentation automation moved this from Reviewer approved to Done Dec 2, 2022
@quarkus-bot quarkus-bot bot added this to the 2.16 - main milestone Dec 2, 2022
@gsmet gsmet modified the milestones: 2.16 - main, 2.15.0.Final Dec 2, 2022
@cescoffier
Copy link
Member Author

@loicmathieu CracConfig was introduced very recently as a placeholder for SnapStart - before we can use that name in public.

It does not remove the CRaC support, it just makes things less confusing, as if you would have used the SnapStart feature with another CRaC impl.... bad things would have happened.

@cescoffier cescoffier deleted the snapstart branch December 3, 2022 14:05
@loicmathieu
Copy link
Contributor

@gsmet @cescoffier I see now that CRaC support is in RunnerClassLoader and this has nothing to do with SnapStart. Thanks for the clarifications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants