Skip to content

Comments

Add FEM#35

Open
cpommier wants to merge 4 commits intomasterfrom
add-fem
Open

Add FEM#35
cpommier wants to merge 4 commits intomasterfrom
add-fem

Conversation

@cpommier
Copy link
Member

@cpommier cpommier commented Sep 17, 2020

Duplicate of #34 , no slashes allowed in branch name in our pipelines.

@cpommier
Copy link
Member Author

cpommier commented Sep 17, 2020

Hi @diegomic,
there is an error when trying to index your endpoint, apparently on the following call the pagination doesn't work:
http://51.145.230.169:8081//brapi/v1/studies?page=0&pageSize=500

@cpommier cpommier mentioned this pull request Sep 17, 2020
Closed
@cpommier cpommier self-assigned this Jan 4, 2021
@diegomic
Copy link

diegomic commented Mar 4, 2021

Hi @cpommier,
In FEM we had to change the url of the brapi server. the new "brapi:endpointUrl" is "http://brapi.fmach.it:8081/brapi/v1/".
I am not sure how I can change the json file attached to this pull. I attach here the new JSON file.
Please let me know if I should I make a new pull request.
Thank you
best regards
Diego Micheletti
FEM.json.zip

@cpommier
Copy link
Member Author

cpommier commented Mar 5, 2021

Hi @diegomic
Thanks, I'll handle the pull request.
There are two improvements you could add to your endoint when possible:

  • "documentationURL":"link to the web page describing this particular study, if ti exists"
  • "studyType": "Phenotyping study"

@diegomic
Copy link

diegomic commented Mar 5, 2021

Thank you @cpommier for the prompt reply.
I will speak with my colleagues to implement the improvements you suggested.

cpommier and others added 4 commits April 12, 2021 10:52
@cpommier
Add FEM
a21cd9d
@cpommier
Fix/macos_multiprocess (#38)
795644f 
* Working fix for macos

* Improved error message on transformation failure

* Raise error with faulty endpoints

* Handle case with unecessary data list for study details call, see comments delimiter in brap.py
@cpommier
Updating FEM endpoint URL
86e616f
@cpommier
If no @type, then defaulting to Phenotyping study. This is likely a t…
2c9c0ef 
…emp workaround
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@diegomic diegomic diegomic approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@cpommier cpommier

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cpommier @diegomic