Skip to content

Updated README and dependencies - #31

Merged
darrenhunt2 merged 21 commits into
testfrom
feature/edit-readme
Jul 12, 2022
Merged

Updated README and dependencies#31
darrenhunt2 merged 21 commits into
testfrom
feature/edit-readme

Conversation

@darrenhunt2

Copy link
Copy Markdown
Contributor

This PR adds the following changes:

  • Adds docstrings to files, along with some other minor code cleanup
  • Removes the unused weather.py file
  • Updates the readme
  • Updates Node and Python dependencies (needs testing)
  • Updates to run with Python 3.9
  • Updates the Github Actions (needs testing)

One thing to note is that the dev stage is still called "api" (check the serverless.yml file). We should decide if we want to change this so that the stage naming convention matches the other repositories. (Additionally, I added in the readme that the base URL has /{stage} at the end, but unsure if this will need to change.)

@kciurleo kciurleo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great, especially the endpoint documentation for such varied endpoints!

Comment thread README.md Outdated
* **Query Params**
```
git clone https://github.com/LCOGT/photonranch-api.git
cd photonranch-calendar

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cd photonranch-api

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

Comment thread README.md

* **Code:** 500 <br />
**Content:** `{ error : "The specified sitecode did not match any config file." }`
Requests for all services in this repository are handled at the base URL `https://api.photonranch.org/{stage}`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that we should change the naming so this is true (and consistent with other repositories) - "dev" for dev, "prod" for prod etc

@timbeccue timbeccue left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great work! Looks good to me.

Comment thread .github/workflows/main.yml Outdated
Comment on lines +11 to +19
staging:
outputs:
deploy-stage: ${{ steps.deploy-stage.outputs.stage }}
runs-on: ubuntu-latest
steps:
- name: Set up deployment stage
id: deploy-stage
run: |
echo "::set-output name=stage::${{ fromJSON('{"main":"prod","dev":"dev","test":"test"}')[github.ref_name] }}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The name 'staging' is already a common word for a specific part of a deployment process. Usually it refers to an environment that behaves like our dev setup. I would recommend changing the name here to avoid confusion. Since it's basically just assigning a value to a variable, maybe something descriptive along those lines.

Comment thread api/handler.py Outdated

First, get a list of files to be zipped based on
the query parameters specified. Next, call a Lambda function
(defined in the repository zip-downloads) that creates a zip

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The repository is now called photonranch-downloads

Comment thread serverless.yml Outdated
authorizerFunc:
handler: authorizer.auth
handler: api/authorizer.auth
cors: true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this line can be removed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The cors: true line, to be clear

@darrenhunt2
darrenhunt2 merged commit 0e016bc into test Jul 12, 2022
@darrenhunt2
darrenhunt2 deleted the feature/edit-readme branch July 12, 2022 20:23
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