Some candidate areas for review

[gmerritt]

Dear RTL Developers,

Here are several areas of this application that might benefit from examination & feedback. There may be plenty more, but these top the list from what I know and understand.

• (1) The python “GCP” bits that give the app its specific function
• (2) Use of AWS Secrets Manager...
  • Instead of local secrets for development
  • Instead of S3 in deployment
• (3) The app’s main Vue page
• (4) The UI / the tool / its use, overall

---

(1) The python “GCP” bits that give the app its specific function

---

https://github.com/ets-berkeley-edu/hartsfield/blob/main/hartsfield/api/datahub_archive_url_fetch.py

Its role:

• Clean & check submitted gs:// url
• Instantiate GCP storage client
• Ask for…
  • …a 7-day “secret” https:// download URL…
  • …for the given gs:// blob reference.
• Return response & status message, whether good or bad

---

(2) Use of AWS Secrets Manager

---

The setup

• Secrets defined in AWS Secrets manager
  • AUTHORIZED_USERS, GCP_JSON_CREDENTIALS
  • https://us-west-2.console.aws.amazon.com/secretsmanager/listsecrets?region=us-west-2
• Environment variables referenced in hartsfield / config / default.py
  • https://github.com/ets-berkeley-edu/hartsfield/blob/main/config/default.py
• Utility python to fetch a Secrets value
  • https://github.com/ets-berkeley-edu/hartsfield/blob/main/hartsfield/api/read_aws_secret.py

Instead of local secrets for development

• https://ucberkeley.awsapps.com/start/#/?tab=accounts -> Access keys
• Run the following commands in your terminal to set the AWS environment variables. Learn more

export AWS_ACCESS_KEY_ID="AS…
export AWS_SECRET_ACCESS_KEY="dWwo…
export AWS_SESSION_TOKEN="IQoJ…

Instead of S3 in deployment

• EB service role
  • RTL-app-hartsfield-dev-secrets-read
  • https://us-east-1.console.aws.amazon.com/iam/home?region=us-west-2#/roles/details/RTL-app-hartsfield-dev?section=permissions

---

(3) The app’s main Vue page

---

Are the formatting and functionality declared & coded appropriately for a Vue page?

https://github.com/ets-berkeley-edu/hartsfield/blob/main/src/views/Fetchurl.vue

---

(4) The UI / the tool / its use, overall

---

As simple as it is, does it present and behave reasonably and according to convention?

Requires campus full tunnel VPN to access, and requires your UID to be allow-listed to authenticate. (Greg can add UIDs for review purposes.)

https://hartsfield-dev.datahub.berkeley.edu/

[gmerritt]

Hey there, everyone!

Shane told me that @remillet suggested that I put some candidate areas of review for Hartsfield into a GitHub issue, so here it is!

I'll also tag @pauline2k and @lyttam for reference, but I'm not sure the precise process here regarding just who / when / how / etc.

If additional and/or different information from me would be helpful, just holler.

Thanks!

[pauline2k]

Thanks @gmerritt. We'll have more devs on hand next week to take a look. Probably what I'll do is set up a workflow with a placeholder PR, like the one described here, so that we can make use of our usual code-commenting practices https://astrofrog.github.io/blog/2013/04/10/how-to-conduct-a-full-code-review-on-github/

[gmerritt]

Note -- re: the astrofrog suggestion for code review (Good stuff! Thanks Pauline!), it seems to me that the first approach, which requires the first commit to be unimportant, is probably not our case.

This puts us in the "If the first commit is important..." case, in which...

... this makes things a little more complicated. The approach we'll take here is to create two new branches - review, containing the code to review, and empty, containing no files - both of which contain a common and empty first commit (which we will add). In this way, the two branches have a common history, even though the empty branch has no files. We can set then set up a pull request from review to empty.

It seems to all break down to a straightforward set of steps, but definitely something to not be done lightly, and with some care & precautions.

I'm willing to jump in on this, but would be happy to know if there are any particularly suggestions or pieces of advice related to this process. Also, could be something to do as a pair of people screen sharing over Zoom, just for a second brain to be involved. :)

[pauline2k]

@gmerritt I think the initial commit might have been placeholder enough that, for review purposes, we might be able to get by with creating a new PR for all subsequent commits. Anyhow I've gone ahead with creating #30, and we'll see how the workflow suits us!

[gmerritt]

Awesome! Thank you, @pauline2k !

[johncrossman]

@gmerritt @pauline2k - I'm jumping in the mix here – I'll put comments to PR #30.

[gmerritt]

Hi @johncrossman and @pauline2k,

Is this a reasonable (or preferred...or discouraged...) way for me to work my way through implementing changes?

1. I start with a local, current Hartsfield (matching what’s live, not John’s suggestions)
2. I begin implementing & testing locally some of the suggested changes
3. I make a new PR with my first set of several changes
4. I (or John) approves PR, I watch Hartsfield redeploy, confirm that it’s working ok in AWS
5. I lather, rinse, repeat with my subsequent changes

Thanks!

[pauline2k]

@gmerritt yes, exactly. PR #30 is just a reference point, so you want to submit your actual changes as separate PRs against the current main branch.