Support Tahoe 2.0 site configuration client by OmarIthawi · Pull Request #63 · appsembler/xblock-launchcontainer

OmarIthawi · 2022-09-16T08:50:47Z

Fix RED-3385. Read Tahoe 2 site from secret on Tahoe 2.0 sites and from site_values on Tahoe 1.0 sites.

Breaking change

Tahoe Hawthorn is no longer supported in v4.0.0 due to the lack of Site Configuration Client. It will act as if it's installed on a Vanilla Hawthorn.

TODO

Test on devstack with site configuration on a Tahoe 2.0 site
- with site-specific value
- with ENV_TOKENS value
Test on devstack with site configuration on a Tahoe 1.0 site
- with site-specific value
- with ENV_TOKENS value

- fix author_view when wharf_url is missing - support rendering templates in tests - new author_view for studio - refactored tests - Tahoe Hawthorn is no longer supported in v4.0.0 due to the lack of Site Configuration Client. It will act as if it's installed on a Vanilla Hawthorn. - Install `tahoe-sites` and `site-configuration-client` to simulate Tahoe environment. - Add support for Tahoe 2.0 site by reading `LAUNCHCONTAINER_WHARF_URL` from Site Config Service `secret`s. Tahoe 1.0 sites will still use `site_values` as usual - Breaking change: Drop support for pre-Juniper Tahoe installations. The last version to support Hawthorn is `3.0.0` need to complicate the XBlock with caching calls. - Simplify reading the Wharf URL * Read from `ENV_TOKENS` for regular Open edX installation (non-Tahoe) * Read from [Site Configuration Client](https://github.com/appsembler/site-configuration-client/) for Juniper+ installations - Refactored unit tests - Refactor testing environments in `tox.ini` to test for `tahoe` and non `tahoe` environments removing `tox-gh-actions`. - Remove complex and low-value caching for `wharf_url`

briandant

many thanks, @OmarIthawi!

briandant · 2022-09-22T15:08:59Z

+ - Install `tahoe-sites` and `site-configuration-client` to simulate Tahoe environment.
+ - Add support for Tahoe 2.0 site by reading `LAUNCHCONTAINER_WHARF_URL` from Site Config Service `secret`s. Tahoe 1.0 sites will still use `site_values` as usual
+- Breaking change: Drop support for pre-Juniper Tahoe installations. The last version to support Hawthorn is `3.0.0`
+need to complicate the XBlock with caching calls.


Suggested change

need to complicate the XBlock with caching calls.

- Do not complicate the XBlock with caching calls.

actually I think you can delete this line—it's duplicated below

briandant · 2022-09-22T15:09:09Z

+
+ - Install `tahoe-sites` and `site-configuration-client` to simulate Tahoe environment.
+ - Add support for Tahoe 2.0 site by reading `LAUNCHCONTAINER_WHARF_URL` from Site Config Service `secret`s. Tahoe 1.0 sites will still use `site_values` as usual
+- Breaking change: Drop support for pre-Juniper Tahoe installations. The last version to support Hawthorn is `3.0.0`


Suggested change

- Breaking change: Drop support for pre-Juniper Tahoe installations. The last version to support Hawthorn is `3.0.0`

- Breaking change: Drop support for pre-Juniper Tahoe installations. The last version to support Hawthorn is `3.0.0`

briandant · 2022-09-22T15:20:26Z


-        return url
+    def get_wharf_delete_url(self, required=True):
+        wharf_url = self.get_wharf_url(required=required)


I'm not sure I totally understand why we check for a wharf url and then return None. I think it's because we want this method (get_wharf_delete_url) to also make it clear that no URL is available. I'm also not sure why we'd return a delete URL if required=False but there is no wharf_url. I just need a docstring or comment here, I think.

briandant · 2022-09-22T15:21:28Z

  <div id="launcher_notification" class="hide"></div>
  {% if enable_container_resetting %}<button type="button" class="ui-priority-primary" id="launcher_reset">Reset lab</button>{% endif %}
-
+{% else %}


Great addition here 👍

briandant · 2022-09-22T15:26:39Z

+{% else %}
+  {# This message is intended to appear Studio for course authors. #}
+  <p><strong>Error: Unable to connect to the Appsembler Virtual Labs API endpoint.</strong></p>
+  <p>Please configure your Appsembler Virtual Labs API endpoint before using this XBlock.</p>


Suggested change

<p>Please configure your Appsembler Virtual Labs API endpoint before using this XBlock.</p>

<p>Please contact <a href="mailto:support@appsembler.com" target="_blank">Appsembler Support</a> so we can configure your Appsembler Virtual Labs API endpoint.</p>

I could be wrong, but I don't think that the customer can configure this on his own, right? My suggested phrasing above captures that. If he can, I think we should talk with Matthew about writing some customer docs on that (or maybe the docs already exist).

OmarIthawi · 2022-09-22T17:27:14Z

Thanks Brian!! I'll follow up on all additions in another PR to avoid GitHub invalidating your review.

clean up and review comments for PR #63

OmarIthawi force-pushed the tahoe_sites_support branch 7 times, most recently from 4a80014 to bc1ce1c Compare September 16, 2022 12:16

OmarIthawi mentioned this pull request Sep 16, 2022

refactor github/tox testing to support tahoe requirements #62

Merged

Base automatically changed from tahoe_sites_support to master September 16, 2022 12:45

OmarIthawi force-pushed the tahoe_support branch 3 times, most recently from 7030a72 to 8532952 Compare September 19, 2022 12:27

OmarIthawi requested review from amirtds, briandant, johnbaldwin, melvinsoft and shadinaif September 19, 2022 13:25

OmarIthawi marked this pull request as ready for review September 19, 2022 13:25

OmarIthawi requested a review from bryanlandia as a code owner September 19, 2022 13:25

OmarIthawi changed the title ~~support tahoe2 site configuration client~~ Support Tahoe 2.0 site configuration client Sep 19, 2022

OmarIthawi force-pushed the tahoe_support branch from d8f4471 to 231d7f7 Compare September 19, 2022 14:40

briandant approved these changes Sep 19, 2022

View reviewed changes

Comment thread launchcontainer/test_settings.py Outdated

briandant reviewed Sep 19, 2022

View reviewed changes

Comment thread launchcontainer/tests.py Outdated

briandant previously approved these changes Sep 19, 2022

View reviewed changes

OmarIthawi dismissed briandant’s stale review via 431ade0 September 20, 2022 08:04

OmarIthawi force-pushed the tahoe_support branch from e228c23 to 74dc07d Compare September 20, 2022 08:21

OmarIthawi added 2 commits September 20, 2022 11:39

changelog for v4.0.0

1a6f8e6

OmarIthawi force-pushed the tahoe_support branch from 74dc07d to 7133ab2 Compare September 20, 2022 08:41

OmarIthawi requested a review from briandant September 22, 2022 12:11

briandant approved these changes Sep 22, 2022

View reviewed changes

OmarIthawi merged commit 43177a3 into master Sep 22, 2022

OmarIthawi deleted the tahoe_support branch September 22, 2022 17:26

OmarIthawi added a commit that referenced this pull request Sep 22, 2022

clean up and review comments for PR #63

c946ea2

OmarIthawi mentioned this pull request Sep 22, 2022

clean up and review comments for PR #63 #65

Merged

OmarIthawi added a commit that referenced this pull request Sep 23, 2022

Merge pull request #65 from appsembler/cleanup_pr_63

1d35b26

clean up and review comments for PR #63

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Tahoe 2.0 site configuration client#63

Support Tahoe 2.0 site configuration client#63
OmarIthawi merged 2 commits intomasterfrom
tahoe_support

OmarIthawi commented Sep 16, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

briandant left a comment

Uh oh!

briandant Sep 22, 2022

Uh oh!

briandant Sep 22, 2022

Uh oh!

briandant Sep 22, 2022

Uh oh!

briandant Sep 22, 2022

Uh oh!

briandant Sep 22, 2022

Uh oh!

briandant Sep 22, 2022

Uh oh!

OmarIthawi commented Sep 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	need to complicate the XBlock with caching calls.
	- Do not complicate the XBlock with caching calls.

	- Breaking change: Drop support for pre-Juniper Tahoe installations. The last version to support Hawthorn is `3.0.0`
	- Breaking change: Drop support for pre-Juniper Tahoe installations. The last version to support Hawthorn is `3.0.0`

	<p>Please configure your Appsembler Virtual Labs API endpoint before using this XBlock.</p>
	<p>Please contact <a href="mailto:support@appsembler.com" target="_blank">Appsembler Support</a> so we can configure your Appsembler Virtual Labs API endpoint.</p>

Conversation

OmarIthawi commented Sep 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking change

TODO

Uh oh!

Uh oh!

Uh oh!

briandant left a comment

Choose a reason for hiding this comment

Uh oh!

briandant Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

briandant Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

briandant Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

briandant Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

briandant Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

briandant Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

OmarIthawi commented Sep 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

OmarIthawi commented Sep 16, 2022 •

edited

Loading