Arch Tea Time: 2020-07-23


"Does everything need to get code reviewed?"

This is routinely asked in feedback on the security training course; I'm in the process of composing a FAQ and would like to hear opinions from the group on this one.

Small doesn’t necessarily mean innocuous.  But being explicit about what kind of small changes we’re okay with would be better than a blanket "trust the developer" statement. For example:

  • Typos in strings

  • Fixing comments

  • Documentation

  • Others things?

(Note: this is specifically for non-PCI-scope - there will be a separate FAQ for PCI-scope)

PCI = Payment Card Industry

  • Standard for data security that all organizations must follow if they access card-holder data.

  • edX’s PCI Scope

    • Payment Page

Security perspective

  • Impetus was for PCI - so took the opportunity to provide a guideline

What should be guidelines for code reviews?

  • Ideas:

    • Trust Judgment - Responsibility of the coder to decide what review and how many reviewers they need to feel confident before merging.

      • Note: coder may not know whether they need a review (unconscious incompetence)

      • This should be accompanied by guardrails since there are always exceptions when a seemingly innocuous change can have significance

    • Still Inform - Communicate post defacto of merging - ensure the team or someone else was notified

What are reviews for?

  • Correctness

    • Major

    • Minor

  • Cross-education

  • Alignment

  • Awareness

Next Steps

  • @Ben Holt (Deactivated) will put together a proposal, may return for further input/feedback

  • Security working group + policy group will make the final decision and update course

ADR for XBlock

  • Still to be decided: cutoff boundary for XBlocks at Unit-level or at Component-level

    • Note: Blockstore would be able to support either level

  • XBlock Storage

    • Multiple “scopes”

      • Content and Settings scopes are stored in MongoDB

      • User scopes are stored in Django models

      • Lesson learned: overusing scopes for different usages that have different life cycles

  • XBlock

    • Lesson learned: overused XBlocks for navigation layer

      • But, rationale: came in super handy when rapid prototyping

  • FYI: Coursera's Plugin API

ADR for Studio/LMS Split

  • May have a wider impact on other teams - since it impacts where data sits

    • For example: boundary between Cohorts (LMS concept) and Content Groups (Studio concept)

  • Idea: do the same for other boundaries, like LMS versus eCommerce, like LMS versus Discovery

    • For example: boundary between LMS (content access) and Discovery (enrollment / track / etc)

Open edX: Time-based releases

  • Would like to return to time-based releases.

    • This is opposed to feature-based releases.

  • The last one (Juniper) was 17-month release.

  • We are proposing a 4-month cadence (so 3x a year)

  • So this means, Koa would get cut in October. Lilac would get cut in February.

  • Note: Ubuntu upgrade - will want to coordinate a consistent version

  • Questions:

    • How does this affect the DEPR process?

    • How does this affect our support process? (supporting 1 version back)

      • Ideas: Community could possibly backport security fixes to previous versions.

    • How can we support multiple versions of the OS for the native installation?

      • Idea: Transition Native to a containerized production installation.