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

Jump to bottom

Move /api/dev/asset to /api/v3_0 #392

Merged
merged 17 commits into from Mar 23, 2022
Merged

Move /api/dev/asset to /api/v3_0 #392

merged 17 commits into from Mar 23, 2022

Conversation

nhoening
Copy link
Contributor

  • move dev asset to v3
  • make sure asset CRUD still works
  • small improvements to viewing & deleting asset in UI

@nhoening nhoening added the API label Mar 18, 2022
@nhoening nhoening added this to the 0.9.0 milestone Mar 18, 2022
@coveralls
Copy link
Collaborator

coveralls commented Mar 18, 2022 •
edited

Pull Request Test Coverage Report for Build 2021914612

  • 59 of 59 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 68.429%
Totals Coverage Status
Change from base Build 2021711825: 0.0%
Covered Lines: 6908
Relevant Lines: 9573
💛 - Coveralls

@nhoening nhoening mentioned this pull request Mar 18, 2022
Merged
Base automatically changed from Issue-389_Publish_SensorDataAPI_documentation to main March 22, 2022 10:25
@nhoening
move asset API from dev to v3
0ca941c 
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
link asset API in v3 docs
a807fdf 
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
point asset crud tests to new asset API location
9893bad 
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
simplify asset view
5647fa8 
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
upgrade asset legacy table, so it is not in the way of a sensor getti…
c73adec 
…ng deleted (asset gets also deleted)

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
fix mock address for testing asset view responds
fa67a43 
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
updating the list of add commands, which needed two deletions and two…
ff2f5b2 
… additions

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
add a changelog entry for API work (can also become multiple)
b9ec4b1 
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
fix get_assets test: now the Supplier has two assets again (after sen…
b75ac2e 
…sor tests landed)

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
docs: update the state of the data model transition
d8af3db 
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
update API changelog
6a93b5a 
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening
remove double fixture
2bc831b 
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Flix6x
Flix6x requested changes Mar 22, 2022
View reviewed changes
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

Very clean PR. I have some small change requests. If you agree to my suggestions, it might be most efficient to just tell me to implement them. Because I experimented locally on my own suggestions, I have most of theses changes stashed already.

flexmeasures/data/migrations/versions/30f7b63069e1_delete_asset_if_sensor_is_deleted.py Show resolved Hide resolved
flexmeasures/api/v3_0/assets.py Show resolved Hide resolved
flexmeasures/api/v3_0/assets.py Outdated Show resolved Hide resolved
flexmeasures/api/v3_0/assets.py Outdated Show resolved Hide resolved
flexmeasures/api/v3_0/assets.py Outdated Show resolved Hide resolved
flexmeasures/api/v3_0/assets.py Outdated Show resolved Hide resolved
@nhoening
Copy link
Contributor Author

Thanks! I made a few comments, but in general I'm very fine for you to implement your suggestions.

@Flix6x
Instantiate schemas outside of endpoint logic to minimize response time
3467732 
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Forbid rather than ignore: id is dump_only and account_id is excluded…
ddfdff3 
… from patching

Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
No trailing slash is a RESTful recommendation
de4e9ac 
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Synchronize documentation
aef4b44 
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Fix API listing for v3_0
526c4c2 
Signed-off-by: F.N. Claessen <felix@seita.nl>
@nhoening
Copy link
Contributor Author

Thanks! Do you need me to do anything? E.g. approve? Tell me if you're happy and I'll merge.

@Flix6x
Copy link
Contributor

Flix6x commented Mar 23, 2022

I'm happy.

@nhoening nhoening merged commit e2f56df into main Mar 23, 2022
@nhoening nhoening deleted the asset-api-to-v3 branch March 23, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Flix6x Flix6x Flix6x requested changes

Assignees
No one assigned
Labels
API
Projects
None yet
Milestone
0.9.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@nhoening @coveralls @Flix6x