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

More docs (mostly infra) and fixing CI configuration #2810

Merged
merged 29 commits into from
Jul 24, 2023
Merged

Conversation

atvaccaro
Copy link
Contributor

@atvaccaro atvaccaro commented Jul 19, 2023

Description

Describe your changes and why you're making them. Please include the context, motivation, and relevant dependencies.

Part of our general documentation focus this sprint.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

How has this been tested?

Include commands/logs/screenshots as relevant.

CI must pass.

Post-merge follow-ups

Document any actions that must be taken post-merge to deploy or otherwise implement the changes in this PR (for example, running a full refresh of some incremental model in dbt). If these actions will take more than a few hours after the merge or if they will be completed by someone other than the PR author, please create a dedicated follow-up issue and link it here to track resolution.

  • No action required
  • Actions required (specified below)
    Deploy to prod

@atvaccaro atvaccaro self-assigned this Jul 19, 2023
@github-actions
Copy link

@atvaccaro atvaccaro changed the title More docs More docs (mostly infra) and fixing CI configuration Jul 20, 2023
@github-actions
Copy link

@atvaccaro atvaccaro marked this pull request as ready for review July 20, 2023 19:02

1. Building branches containing only a subset of repository contents (for example, a branch only including infra-related code)
* This action is called "projection"
2. Bringing in contents from another repository without relying on published artifacts such as Helm charts
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a little concerned about this re: handoff. cc @themightychris

Do we think it's appropriate or sustainable to hand off the project with this kind of dependency, particularly through an uncommon dependency mechanism (i.e., declared in places that are not necessarily standard/super visible to less experienced users)?

Copy link
Contributor

@themightychris themightychris Jul 21, 2023

Choose a reason for hiding this comment

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

Yeah but the alternatives aren't super great, they'd shift complexity to floating around in the cloud state and in people's heads rather than being baked into GitHub workflows. The goal of these architectures were expressly to solve for how to make the workflows live more inside GitHub and be more "solid state" so their more resilient against long-term churn in the ecosystem and handoffs between contributors. The lower adoption of this approach is of course a concern, but adding ArgoCD or Flux to the mix instead isn't an obvious win either—nor is keeping it all in GitHub Actions but importing tons of complex bash code into the local project to avoid dependencies.

Copy link
Contributor Author

@atvaccaro atvaccaro Jul 21, 2023

Choose a reason for hiding this comment

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

I think we could at least consider referencing the published helm charts rather than bring in the jarvus cluster template. Branch projections are more "obvious" IMO and matters for the gitops workflows. I think the main concern would be upstream changes (e.g. cluster-template) sneaking into cal-itp deploys without someone realizing; that's only relevant for cluster-template not for Actions.

.holo/README.md Outdated Show resolved Hide resolved
apps/maps/README.md Outdated Show resolved Hide resolved
docs/services/gtfs-ckan-uploader.md Show resolved Hide resolved
@lauriemerrell
Copy link
Contributor

Also not sure if/how this should interact with this existing docs page: https://docs.calitp.org/data-infra/infrastructure/README.html

@atvaccaro
Copy link
Contributor Author

Also not sure if/how this should interact with this existing docs page: https://docs.calitp.org/data-infra/infrastructure/README.html

Ah good call, I'll move those where appropriate

Copy link
Contributor

@lauriemerrell lauriemerrell left a comment

Choose a reason for hiding this comment

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

I am approving but I still wonder how things here interact with this section on the docs page I linked before

image

@atvaccaro
Copy link
Contributor Author

Ah good catch thanks, I forgot to remove the old docs.

@atvaccaro atvaccaro merged commit 00d5729 into main Jul 24, 2023
@atvaccaro atvaccaro deleted the more-docs branch July 24, 2023 18:36
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.

3 participants