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

Supports echo.net variant #38 #39

Merged
merged 4 commits into from
Jul 2, 2021

Conversation

BenThurber
Copy link
Contributor

I changed references of echo360.org[.xx] to echo360.net[.xx] to fix Issue #38. I also modified one of the screenshots in the README.md to use .net. I haven't tested it extensively but it works on my echo360 which has the domain echo360.net.au.

Copy link
Owner

@soraxas soraxas left a comment

Choose a reason for hiding this comment

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

Thanks @BenThurber for your PR and for updating all the references to .org within the code.

However, I think the transition is not actually the entire system migrating to a different TLD (as I can still access echo360.org directly; e.g., https://login.echo360.org/) and it might also be localised (e.g. localise to certain country, e.g. .au?). Therefore, if we migrate the entire implementation to .net it will break other existing systems that uses .org.

I checked whois and their registered date and server are still very much active on both TLD.


EDIT: For example, entering echo360.org.au does indeed redirects to echo360.net.au. But entering some other country code, e.g., echo360.org.uk will stays in .org without redirects.


Either way, the fix is simple and I've added notes on where it should check for both TLD (.net and .org). Please update the PR and revert all other non-essential changes as I think other references to .org are still valid and are not essential to make the script works on echo360.net. The only relevant files are README.md and echo360/main.py.

@@ -203,14 +203,14 @@ https://$(hostname)/ess/portal/section/$(UUID)
```
or
```shell
https://echo360.org[.xx]/
https://echo360.net[.xx]/
Copy link
Owner

Choose a reason for hiding this comment

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

Include both lines as both hostname are still active, i.e.,

https://echo360.org[.xx]/
# or with a dot net variant
https://echo360.net[.xx]/

echo360/main.py Outdated
@@ -254,7 +254,7 @@ def main():

setup_logging(enable_degbug)

if not usingEcho360Cloud and "echo360.org" in course_hostname:
if not usingEcho360Cloud and "echo360.net" in course_hostname:
Copy link
Owner

Choose a reason for hiding this comment

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

Change the condition to

if not usingEcho360Cloud and any(token in course_hostname for token in ["echo360.org", "echo360.net"]):
   ...

@BenThurber
Copy link
Contributor Author

I reverted the code and documentation changes, and just made the changes you recommended. I left the change to the screenshot since that was for an echo.net.au domain.

@soraxas soraxas changed the title org to net change, fix Issue #38 Supports echo.net variant #38 Jul 2, 2021
@soraxas soraxas merged commit 71c21f3 into soraxas:master Jul 2, 2021
@soraxas
Copy link
Owner

soraxas commented Jul 2, 2021

Thanks!

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.

2 participants