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

Create inverter accessing points #80

Merged
merged 24 commits into from
Apr 21, 2023
Merged

Conversation

neha-vard
Copy link

@neha-vard neha-vard commented Apr 13, 2023

Pull Request

Description

Create the following endpoints:

/sites/<site_uuid>/inverters
/inverters

  • Returns the inverter data from the inverters linked to this site. If there isn’t any, return a 404.
  • Returns the inverter data for all of the authenticated client’s inverters.
  • Create a get_inverters_for_client function to get the inverters.
  • Created pydantic models for the inverter data responses.
  • Made test for both inverter endpoints.

Fixes #71

@AndrewLester AndrewLester changed the base branch from main to h4i/enode April 17, 2023 22:21
@neha-vard neha-vard marked this pull request as ready for review April 18, 2023 03:12
Copy link
Contributor

@AndrewLester AndrewLester left a comment

Choose a reason for hiding this comment

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

A few things here, but overall very nice job.

pv_site_api/main.py Outdated Show resolved Hide resolved
pv_site_api/main.py Outdated Show resolved Hide resolved
pv_site_api/main.py Outdated Show resolved Hide resolved
pv_site_api/main.py Outdated Show resolved Hide resolved
assert client is not None

async with httpx.AsyncClient() as httpxClient:
headers = {"Enode-User-Id": client.client_uuid}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

pv_site_api/main.py Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@ericcccsliu ericcccsliu left a comment

Choose a reason for hiding this comment

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

Great work getting this all up

pv_site_api/fake.py Outdated Show resolved Hide resolved
pv_site_api/main.py Outdated Show resolved Hide resolved
pv_site_api/main.py Outdated Show resolved Hide resolved
pv_site_api/main.py Outdated Show resolved Hide resolved
Comment on lines 14 to 19
# def test_get_inverters(db_session, client, forecast_values):
# response = client.get(f"/sites/{site_uuid}/clearsky_estimate")
# assert response.status_code == 200

# clearsky_estimate = ClearskyEstimate(**response.json())
# assert len(clearsky_estimate.clearsky_estimate) > 0
Copy link
Contributor

@AndrewLester AndrewLester Apr 18, 2023

Choose a reason for hiding this comment

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

Next, we'll want to uncomment this test (and change the endpoint in it). Here's the issue: your endpoints will end up trying to make a request to Enode, but we shouldn't do that in the test. Instead, you'll use https://pypi.org/project/pytest-httpx/ to have the Enode API requests get intercepted and swapped with fake data. Read the docs there for info, then add the library via poetry add -D pytest-httpx. You'll first need to bump the version of httpx in the pyproject.toml to 0.24.0.

Then, add this code to conftest.py:

@pytest.fixture
def non_mocked_hosts() -> list:
    return ["testserver"]

Then, in your tests, add httpx_mock as a parameter (to the non fake ones) and use it to mock a Enode request:

httpx_mock.add_response(url="https://enode-api.production.enode.io/inverters")

To actually set the data in the mocked response, follow the docs for the library.

@codecov
Copy link

codecov bot commented Apr 20, 2023

Codecov Report

Merging #80 (b0a2e6c) into h4i/enode (a506477) will increase coverage by 2.11%.
The diff coverage is 98.43%.

@@              Coverage Diff              @@
##           h4i/enode      #80      +/-   ##
=============================================
+ Coverage      84.82%   86.93%   +2.11%     
=============================================
  Files              9        9              
  Lines            336      398      +62     
=============================================
+ Hits             285      346      +61     
- Misses            51       52       +1     
Impacted Files Coverage Δ
pv_site_api/main.py 90.00% <94.73%> (+0.52%) ⬆️
pv_site_api/_db_helpers.py 100.00% <100.00%> (ø)
pv_site_api/fake.py 100.00% <100.00%> (ø)
pv_site_api/pydantic_models.py 100.00% <100.00%> (ø)
pv_site_api/utils.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@ericcccsliu ericcccsliu left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@AndrewLester AndrewLester left a comment

Choose a reason for hiding this comment

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

Nice

@anyaparekh anyaparekh merged commit 1a23431 into h4i/enode Apr 21, 2023
@anyaparekh anyaparekh deleted the ap-nv/inverter-endpoints branch April 21, 2023 02:28
@AndrewLester AndrewLester restored the ap-nv/inverter-endpoints branch April 23, 2023 17:31
@AndrewLester AndrewLester deleted the ap-nv/inverter-endpoints branch April 23, 2023 17:31
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.

Create inverter accessing endpoints
4 participants