Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

feat: install atlas in mfe role FC-0012 #6968

Closed
wants to merge 1 commit into from

Conversation

OmarIthawi
Copy link
Contributor

Description

Download the openedx-atlas executable for all Micro-frontends to be used in make pull_translations

This installs atlas on all Ansible build targets such as Native Installation and Devstack.

Once merged it will make the atlas CLI executable available on all Micro-frontend Dockerfiles and native installation such as Profile, Account, Learning and others. It will make the following command possible: atlas pull which replaces transifex pull as part of the Translation Infrastructure update OEP-58 project.

Testing

  • Re-build a devstack docker image and run atlas pull

Configuration Pull Request

Make sure that the following steps are done before merging:

  • A SRE team member has approved the PR if it is code shared across multiple services and you don't own all of the services.
  • Are you adding any new default values that need to be overridden when this change goes live? [Omar] No, the default value is safe to be used.
    • Update the appropriate internal repo (be sure to update for all our environments)
    • If you are updating a secure value rather than an internal one, file a SRE ticket with details.
    • Add an entry to the CHANGELOG.
  • If you are making a complicated change, have you performed the proper testing specified on the Ops Ansible Testing Checklist? Adding a new variable does not require the full list (although testing on a sandbox is a great idea to ensure it links with your downstream code changes).
  • Think about how this change will affect Open edX operators. Have you updated the wiki page for the next Open edX release?
    • [Omar] The documentation update is work-in-progress.

References

This contribution is part of the FC-0012 project which is sparked by the Translation Infrastructure update OEP-58.

Up-to-date project overview and details are available in the Approach Memo and Technical Discovery: Translations Infrastructure Implementation document.

Join the conversation on Open edX Slack #translations-project-fc-0012.

Check the links above for full information about the overall project.

Internalization is being rearchitected in Open edX Python, XBlock, Micro-frontend, and other projects. There are a number of immediately visible changes:

  • Remove source and language translations from the repositories, hence no .json, .po or .mo files will be committed into the repos.
  • Add standardized make extract_translations in all repositories
  • Push user-facing messages strings into openedx/openedx-translations.
  • Integrate root repositories with openedx/openedx-atlas to pull translations on build/deploy time

Breaking Changes

One of the primary goals of the project is to avoid breaking changes. If you noticed any suspicious code, please raise your concern. But before that, please know the strategy we're following to avoid breaking changes:

  • All variable have sane-defaults.
  • The changes are backward compatible and don't break any existing deployments.

@openedx-webhooks
Copy link

openedx-webhooks commented Aug 3, 2023

Thanks for the pull request, @OmarIthawi! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 3, 2023
@OmarIthawi OmarIthawi force-pushed the atlas-mfe branch 2 times, most recently from 0b0b81b to 2183500 Compare August 6, 2023 17:29
@OmarIthawi OmarIthawi marked this pull request as draft August 7, 2023 11:54
@brian-smith-tcril
Copy link
Contributor

I think we should wait until we implement openedx/openedx-atlas#34 and use npm here

Install atlas on all Micro-frontends to be used in `make pull_translations`

This installs `atlas` on all Ansible build targets such as Native Installation and Devstack.

This contribution is part of the [FC-0012 project](https://openedx.atlassian.net/l/cp/XGS0iCcQ) which is sparked by the [Translation Infrastructure update OEP-58](https://open-edx-proposals.readthedocs.io/en/latest/architectural-decisions/oep-0058-arch-translations-management.html#specification).
@OmarIthawi OmarIthawi changed the title feat: install atlas in mfe role feat: install atlas in mfe role FC-0012 Oct 13, 2023
@brian-smith-tcril
Copy link
Contributor

@OmarIthawi do we know the status on this one? Is this still something we're planning on doing or can it be closed?

@OmarIthawi
Copy link
Contributor Author

@brian-smith-tcril this is waiting for both: 1) Me completing the edx-platform work to have free time for this, 2) 2U input on how should we copy/mimic the Tutor MFE atlas pull logic into configurations.

I think we can start with (2) and ping someone from 2U. The review input document we have doesn't list the configuration repo.

@kdmccormick
Copy link
Contributor

I'm noticing that this is would be the only NPM package which we'd specifically install into the playbook. I imagine we'll need to make sure the same version is also installed into Tutor.

All other build requirements are installed transitively via frontend-build, I believe (correct me if I'm wrong). So, instead of installing Atlas here, would it be possible to install it via frontend-build as well?

@brian-smith-tcril
Copy link
Contributor

in overhangio/tutor-discovery#41 we install atlas via pip

@kdmccormick
Copy link
Contributor

Right, okay. I was going to say that these requirements should all go into package.json and requirements/yada.in like everything else, but now that I'm thinking about it more, I guess the client of Atlas is not the upstream repo itself, but rather the builder of the repo. In other words, Tutor and Ansible are the users of Atlas, so they will each maintain their own version pins and handle Atlas upgrades separately.

You folks probably came to this conclusion a while ago. Sorry for the distraction. This change LGTM.

@OmarIthawi
Copy link
Contributor Author

OmarIthawi commented Nov 18, 2023

@kdmccormick @brian-smith-tcril as far as the place to install the openedx-atlas package is concerned the current approach is to install openedx-atlas in each repository in order to benefit from the automatic bot package updates (example) and to cause the least surprise to future maintainers looking to change the packages.

in overhangio/tutor-discovery#41 we install atlas via pip

This was the first attempt to integrate atlas into a builder, and we should remove this pin to avoid confusion now course-discovery has its requirement.

I'm keeping this pull request merely as a "todo" item, but I'll close it and re-open once I'm done with the edX Platform work:

I've created a pull request to remove the redundant install:

@OmarIthawi OmarIthawi closed this Nov 18, 2023
@openedx-webhooks
Copy link

@OmarIthawi Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

1 similar comment
@openedx-webhooks
Copy link

@OmarIthawi Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants