Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

use universal runtime for embeds #1056

Merged
merged 4 commits into from
Apr 22, 2021
Merged

use universal runtime for embeds #1056

merged 4 commits into from
Apr 22, 2021

Conversation

trieloff
Copy link
Contributor

No description provided.

@github-actions
Copy link

This PR will trigger a patch release when merged.

@tripodsan tripodsan self-requested a review April 21, 2021 00:48
},
"DATA_EMBED_SERVICE": {
"type": "string",
"description": "URL of a DataEmbed Service that takes the appended URL and returns an iterable JSON representation.",
"default": "https://adobeioruntime.net/api/v1/web/helix/helix-services/data-embed@v1"
"default": "https://helix-pages.anywhere.run/helix-services/data-embed@v2"
Copy link
Contributor

@tripodsan tripodsan Apr 21, 2021

Choose a reason for hiding this comment

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

this doesn't really work for non-universal actions, since the version lock only works for runtime actions.
for universal, this is hardcoded:

uri = resolver.createURL({
package: 'helix-services',
name: 'data-embed',
version: 'v2',
});

so I would revert the data-embed change.

also see #1058

Copy link
Contributor

Choose a reason for hiding this comment

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

I added a commit to revert the data-embed change

@codecov
Copy link

codecov bot commented Apr 21, 2021

Codecov Report

Merging #1056 (41b9276) into main (e0b740a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main     #1056   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           60        60           
  Lines         1602      1602           
=========================================
  Hits          1602      1602           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update faeceaf...41b9276. Read the comment docs.

},
"DATA_EMBED_SERVICE": {
"type": "string",
"description": "URL of a DataEmbed Service that takes the appended URL and returns an iterable JSON representation.",
"default": "https://adobeioruntime.net/api/v1/web/helix/helix-services/data-embed@v1"
"default": "https://adobeioruntime.net/api/v1/web/helix/helix-services/data-embed@v2"
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment below

@trieloff
Copy link
Contributor Author

Made obsolete by #1057

@trieloff trieloff closed this Apr 21, 2021
@tripodsan
Copy link
Contributor

Made obsolete by #1057

why? #1057 removed spark as embed. but this one uses the correct embed service in general.
unless we remove it as suggested in #1060

@trieloff trieloff reopened this Apr 21, 2021
@trieloff trieloff changed the title fix(embed): disable spark embeds, use universal runtime for embeds use universal runtime for embeds Apr 21, 2021
@tripodsan tripodsan self-requested a review April 21, 2021 09:00
@trieloff
Copy link
Contributor Author

https://circleci.com/workflow-run/f0d0b24f-e666-4b27-873f-e684f7c6ccbe The smoke test failures seem unrelated.

trieloff added a commit to adobe/helix-cli that referenced this pull request Apr 21, 2021
adobe/helix-pipeline#1056 switches the embed service URL to universal runtime. This means that some of the recordings for the integration tests need to be re-downloaded, as the URL has changed. As helix-embed has evolved in the meantime, this also means that the test output has changed a bit. There is a good chance that we will end up in a smoke test deadlock, as this PR won't pass the tests until adobe/helix-pipeline#1056 has been merged and vice versa.
@trieloff trieloff merged commit 2516e50 into main Apr 22, 2021
@trieloff trieloff deleted the do-not-embed-spark branch April 22, 2021 08:34
@adobe-bot
Copy link

🎉 This PR is included in version 13.9.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

tripodsan pushed a commit to adobe/helix-cli that referenced this pull request Apr 23, 2021
#1745)

adobe/helix-pipeline#1056 switches the embed service URL to universal runtime. This means that some of the recordings for the integration tests need to be re-downloaded, as the URL has changed. As helix-embed has evolved in the meantime, this also means that the test output has changed a bit. There is a good chance that we will end up in a smoke test deadlock, as this PR won't pass the tests until adobe/helix-pipeline#1056 has been merged and vice versa.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants