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

feat: added endpoint to return .json for the input .tiff file #185

Merged
merged 6 commits into from
Oct 2, 2022
Merged

feat: added endpoint to return .json for the input .tiff file #185

merged 6 commits into from
Oct 2, 2022

Conversation

khanjasir90
Copy link
Contributor

@khanjasir90 khanjasir90 commented Sep 16, 2022 •
edited
Loading

Signed-off-by: khanjasir90 khanmohdjasir@gmail.com

Pull Request Template

Description

This PR creates a /getConfig endpoint which would return the .json file for the corresponding .tiff file.
URL:http://localhost:8080/gcbm/getConfig
Curl : curl -d "title=run4&file_name=Classifier1_moja" -X POST http://localhost:8080/gcbm/getConfig

Fixes # (issue) #183

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Additional Context (Please include any Screenshots/gifs if relevant)

Output :
image

...

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings
  • I have tagged the reviewers in a comment below incase my pull request is ready for a review
  • I have signed the commit message to agree to Developer Certificate of Origin (DCO) (to certify that you wrote or otherwise have the right to submit your contribution to the project.) by adding "--signoff" to my git commit command.

@khanjasir90
feat: added endpoint to return .json for the input .tiff file
9bdd967 
Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>
@khanjasir90
Copy link
Contributor Author

@Namyalg could u pls take a look at this one!

@Namyalg
Copy link
Member

Namyalg commented Sep 17, 2022 •
edited
Loading

Hi @khanjasir90, looks good.
A few observations :

  1. Running the command with a file that does not exist, throws the error simulation does not exist, I think this needs to be changed, the error should report that the file does not exist

image

  1. Can you add snapshots, or list some of the other edge cases you have tried?

  2. What if a file which does not have a JSON is supplied, eg : the gcbm_input.db file ?

@Namyalg
Copy link
Member

Namyalg commented Sep 17, 2022

CC : @SanjaySinghRajpoot @padmajabhol

Namyalg
Namyalg reviewed Sep 17, 2022
View reviewed changes
local/rest_api_gcbm/app.py Outdated

# Check if simulation exists or not
if not os.path.exists(f"{input_dir}"):
return {"error": "Simulation does'nt exist"}, 400
Copy link
Member

@Namyalg Namyalg Sep 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Change to "Simulation with name " + title + " does not exist"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Namyalg I'll do the above mentioned change!

@khanjasir90
Copy link
Contributor Author

What if a file which does not have a JSON is supplied, eg : the gcbm_input.db file ?

  • Since we are taking the file without any extension then it will throw file does'nt exist for the given simulation.
    I'll add some ss related to edge cases asap in.

@SanjaySinghRajpoot
Copy link
Contributor

SanjaySinghRajpoot commented Sep 18, 2022 •
edited
Loading

Makes sense, the frontend would read the data from this file (.tiff) with the help of this endpoint. The data will be then displayed to the user and with another endpoint that is the /setConfig the data will be updated on the backend. So if the API is unable to find the file we need to send a specific response message to the frontend. @Namyalg points are correct just make the changes and then we are good to go!

@khanjasir90
feat: updated endpoint with specified changes and some constraints
2501aa4 
Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>
@khanjasir90
feat: formatted code with black
3f2dcc2 
Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>
@khanjasir90
Copy link
Contributor Author

@Namyalg @SanjaySinghRajpoot I have made the specified changes and also added some strict constraints.pls have a look at it and share what are your thoughts on the same. I have also add snapshots of the edge cases for which I have exec the endpoint.
Edge Case 1 : No Simulation name specified
nosimulation
Edge Case 2: No File name specified
nofilename
Edge Case 3: Wrong File Name
wrongfilename
EdgeCase 4: Wrong Simulation Name
wrongsimulation
Correct Output:
correct

Namyalg
Namyalg reviewed Oct 2, 2022
View reviewed changes
Copy link
Member

@Namyalg Namyalg left a comment

Choose a reason for hiding this comment

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

The various scenarios are handled well.
Thank you @khanjasir90
Once these minor changes are done, it will be ready for merge!

local/rest_api_gcbm/app.py Outdated

# Check if simulation exists or not
if not os.path.exists(f"{input_dir}"):
return {"error": "Simulation with the name " + title + " does'nt exists"}, 400
Copy link
Member

Choose a reason for hiding this comment

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

Change to doesn't

local/rest_api_gcbm/app.py Outdated
input_dir_file = f"{input_dir}/{file_name}.json"
# Check if file exists or not
if not os.path.exists(f"{input_dir_file}"):
return {"error": "File with name " + file_name + " does'nt exists"}, 400
Copy link
Member

Choose a reason for hiding this comment

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

Change to doesn't

@khanjasir90
updated minor typos
0cee49a 
Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>
@khanjasir90
Merge branch 'feat/creat-endpoint-to-return-json' of https://github.c…
e5424ff 
…om/khanjasir90/FLINT.Cloud into feat/creat-endpoint-to-return-json
@khanjasir90
Copy link
Contributor Author

Thanks @Namyalg ! I have made the appropriate changes :)

@Namyalg Namyalg merged commit 42167ad into moja-global:master Oct 2, 2022
Freeman-kuch pushed a commit to Freeman-kuch/FLINT.Cloud that referenced this pull request Oct 20, 2022
@khanjasir90 @Namyalg @Freeman-kuch
feat: added endpoint to return .json for the input .tiff file (moja-g…
3268614 
…lobal#185)

* feat: added endpoint to return .json for the input .tiff file

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* feat: updated endpoint with specified changes and some constraints

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* feat: formatted code with black

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* updated minor typos

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>
Co-authored-by: Namya LG <53875297+Namyalg@users.noreply.github.com>
Signed-off-by: Freeman <freemanoparaocha@gmail.com>
Freeman-kuch pushed a commit to Freeman-kuch/FLINT.Cloud that referenced this pull request Oct 20, 2022
@khanjasir90 @Namyalg @Freeman-kuch
feat: added endpoint to return .json for the input .tiff file (moja-g…
0dbed6d 
…lobal#185)

* feat: added endpoint to return .json for the input .tiff file

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* feat: updated endpoint with specified changes and some constraints

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* feat: formatted code with black

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* updated minor typos

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>
Co-authored-by: Namya LG <53875297+Namyalg@users.noreply.github.com>
Signed-off-by: Freeman <freemanoparaocha@gmail.com>
Freeman-kuch pushed a commit to Freeman-kuch/FLINT.Cloud that referenced this pull request Oct 20, 2022
@khanjasir90 @Namyalg @Freeman-kuch
feat: added endpoint to return .json for the input .tiff file (moja-g…
ba530db 
…lobal#185)

* feat: added endpoint to return .json for the input .tiff file

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* feat: updated endpoint with specified changes and some constraints

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* feat: formatted code with black

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* updated minor typos

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>
Co-authored-by: Namya LG <53875297+Namyalg@users.noreply.github.com>
Signed-off-by: Freeman <freemanoparaocha@gmail.com>
Freeman-kuch pushed a commit to Freeman-kuch/FLINT.Cloud that referenced this pull request Oct 28, 2022
@khanjasir90 @Namyalg @Freeman-kuch
feat: added endpoint to return .json for the input .tiff file (moja-g…
7ecf76a 
…lobal#185)

* feat: added endpoint to return .json for the input .tiff file

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* feat: updated endpoint with specified changes and some constraints

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* feat: formatted code with black

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

* updated minor typos

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>

Signed-off-by: khanjasir90 <khanmohdjasir@gmail.com>
Co-authored-by: Namya LG <53875297+Namyalg@users.noreply.github.com>
Signed-off-by: Freeman <freemanoparaocha@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Namyalg Namyalg Namyalg approved these changes

@SanjaySinghRajpoot SanjaySinghRajpoot SanjaySinghRajpoot left review comments

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

Successfully merging this pull request may close these issues.
3 participants
@khanjasir90 @Namyalg @SanjaySinghRajpoot