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

chore: update for 2023 #29

Merged
merged 11 commits into from
Aug 3, 2023
Merged

chore: update for 2023 #29

merged 11 commits into from
Aug 3, 2023

Conversation

digicosmos86
Copy link
Contributor

What was updated:

  • All software versions, including modules that this one depends on
  • Added typing for variables that didn't already have it
  • Updated precommit and github action yaml files

@digicosmos86 digicosmos86 linked an issue Jul 19, 2023 that may be closed by this pull request
Copy link
Member

@mirestrepo mirestrepo left a comment

Choose a reason for hiding this comment

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

Looks good to me, main question is about NFS code that seemed to be removed

- uses: actions/checkout@v3
with:
clean: false #this is to avoid permissions on self-hosted runner
- name: Pre-check dirty dirs in self-hosted
Copy link
Member

Choose a reason for hiding this comment

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

lets do this on another PR, but our self-hosted runners are now ephemeral and all of this checking/cleaning of files and directories can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds great! Will add another PR to address this

name: gcr.io/jupyterhub-docker-images/jupyterhub-scipy
tag: phys1600-spring2020
name: gcr.io/jupyterhub-docker-images/mpa2065
tag: latest
Copy link
Member

Choose a reason for hiding this comment

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

best to use another tag that is more stable than latest, but fine for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good 👍

@@ -67,12 +67,6 @@ module "shared-nfs" {

locals {
helm_release_wait_condition = length(kubernetes_secret.tls_secret) > 0 ? kubernetes_secret.tls_secret[0].metadata[0].name : kubernetes_namespace.jhub.metadata[0].name
Copy link
Member

Choose a reason for hiding this comment

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

hm - why did this nfs code change? We also could check this deployment, which is not part of the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I got rid of this for testing and somehow forgot to add it back? I will add it back and rerun the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh @mirestrepo actually tflint complained that this is declared by unused. It was used in line 220 of this file, which was commented out, so I just removed it. Should be safe to leave out then?

@digicosmos86 digicosmos86 merged commit 71d5fe6 into main Aug 3, 2023
3 checks passed
@digicosmos86 digicosmos86 deleted the 28-chore-update-for-2023 branch August 3, 2023 15:55
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.

chore: update for 2023
2 participants