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.

Sign up for GitHub

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

Add measurement processing workflow #363

Merged
42 commits merged into from
Apr 11, 2020
Merged

Add measurement processing workflow #363

42 commits merged into from
Apr 11, 2020

Conversation

jackxujh
Copy link
Contributor

@jackxujh jackxujh commented Apr 10, 2020 •
edited by ghost

This pull request introduces a new workflow for creating measurements in the database. It will take the received measurement from the API or Simulator, and process it, followed by saving to the database.

This PR now includes:

  • Measurement processing workflow design based on discussion in class.
  • Brand new and updated tests that ensure functionality works as expected.

@jackxujh
Add incoming measurement processing workflow
3ea6d35
@jackxujh
Remove unused Grafana dashboard export
589ae59
@jackxujh
Remove unused presets
0719bd0
@jackxujh
Add delegate to create user custom sensor types
7f4e645
@jackxujh
Move files
e3db4fd
@jackxujh
Update tests
036381a
@jackxujh
Format code
8e42c05
@jackxujh
Merge branch 'backend/develop' into backend/formula-processing
e7fe93a 
# Conflicts:
#	mercury/forms.py
#	mercury/views/sensor.py
@jackxujh
Format code
8946cef
@jackxujh
Merge branch 'backend/develop' into backend/formula-processing
bea61a4 
# Conflicts:
#	ag_data/tests/test_simulator.py
@jackxujh
Merge branch 'backend/develop' into backend/formula-processing
d00b481
@jackxujh
Format code
49d5181
@jackxujh
Update built-in sensor types
83af3ef
@jackxujh
Use unified return data type in ingestion engine
6a4f5d9
@jackxujh
Format foreign code
feddbbf
@jackxujh
Update createOrResetBuiltInSensorTypeAtPresetIndex
89cbe73
@jackxujh
Format code
d430886
@jackxujh
Add unit tests for utility functions
3c55c8a
@jackxujh
Add a test for createOrResetAllBuiltInSensorTypes
2a22f81
@jackxujh
Format code
1ed4c8c
@jackxujh
Remove unused tests
ccc161d
@jackxujh
Rename built_in_sensor_types to built_in_content
4e5ed41
@jackxujh
Add tests for createCustomSensorType
0ae5513
@jackxujh
Update import
e954a78
@jackxujh
Format code
61cc365
@jackxujh
Rename test for createCustomSensorType
4b19826
@jackxujh
Revert "Rename test for createCustomSensorType"
f04263a 
This reverts commit 4b19826.
@jackxujh
Add test for createSensorFromPresetAtIndex
b400d2d
@jackxujh
Test multiple sensor creation
5106b30
@jackxujh
Add test for createVenueFromPresetAtIndex
98815b6
@alldne alldne self-requested a review April 11, 2020 00:40
@ghost
Copy link

ghost commented Apr 11, 2020

+702 -457 is a big merge. Can you break this up into smaller, easier to review merges?

@ghost ghost closed this Apr 11, 2020
@jackxujh
Copy link
Contributor Author

jackxujh commented Apr 11, 2020 •
edited

@ray310 This pull request is not small. However, this pull request consists of only one functionality. And because of this functionality, many tests need to be refactored, otherwise they will break. In this sense, this pull request consist of one "logical" change. I am not sure how I can split it into smaller pull requests. Do you think there are some ways to split it up that I overlooked?

@jackxujh jackxujh reopened this Apr 11, 2020
@MichaelLally
Copy link
Contributor

I'm going through to review this now

alldne
alldne suggested changes Apr 11, 2020
View reviewed changes
ag_data/formulas/ingestion_engine.py Show resolved Hide resolved
ag_data/formulas/ingestion_engine.py Show resolved Hide resolved
ag_data/formulas/ingestion_engine.py Show resolved Hide resolved
ag_data/formulas/library/system/mercury_formulas.py Outdated Show resolved Hide resolved
ag_data/formulas/library/system/mercury_formulas.py Show resolved Hide resolved
ag_data/formulas/library/system/mercury_formulas.py Outdated Show resolved Hide resolved
ag_data/formulas/library/system/mercury_formulas.py Outdated Show resolved Hide resolved
ag_data/presets/helpers.py Show resolved Hide resolved
ag_data/presets/helpers.py Show resolved Hide resolved
ag_data/simulator.py Show resolved Hide resolved
MichaelLally
MichaelLally approved these changes Apr 11, 2020
View reviewed changes
Copy link
Contributor

@MichaelLally MichaelLally left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost ghost merged commit 55b3330 into gcivil-nyu-org:backend/develop Apr 11, 2020
@jackxujh
Copy link
Contributor Author

@ray310 Ray, could we revert this PR for now, and merge it later today, if you are not in a hurry to use its functionality? This pull request has been requested for changes.

@jackxujh jackxujh mentioned this pull request Apr 11, 2020
Closed
@jackxujh
Copy link
Contributor Author

@alldne Thank you for the heartfelt review and feedback!

Most comments you provided now have some corresponding code changes either within the merger of this PR, or in #376. Please check if the fixes are sound.

On the other hand, the 3 remaining conversations are now moved to separate issues due to larger footprints to fix.

@jackxujh jackxujh linked an issue Apr 12, 2020 that may be closed by this pull request
Create a workflow for measurement processing, with a preset of flow sensor #120
Closed
@jackxujh jackxujh mentioned this pull request Apr 12, 2020
Open
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alldne alldne alldne requested changes

@MichaelLally MichaelLally MichaelLally approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create a workflow for measurement processing, with a preset of flow sensor
3 participants
@jackxujh @MichaelLally @alldne