search

FAIRDataPipeline / data-registry

The FAIR Data Registry is a Django website and REST API which is used by the FAIR Data Pipeline to store metadata about code runs and their inputs and outputs
BSD 2-Clause "Simplified" License
0 stars 1 forks source link

Remove Port Restrictions #191

Closed RyanJField closed 1 year ago

RyanJField commented 2 years ago

Container systems such as docker require ports to be bound to specific / all interfaces i.e. 0.0.0.0:8000. To Resolve this I have added an address flag -a to the start scripts.

codecov[bot] commented 2 years ago

Codecov Report

Merging #191 (1a82b6b) into main (d4669ec) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #191   +/-   ##
=======================================
  Coverage   87.79%   87.79%           
=======================================
  Files          33       33           
  Lines        2581     2581           
=======================================
  Hits         2266     2266           
  Misses        315      315
Flag Coverage Δ
unittests 87.79% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d4669ec...1a82b6b. Read the comment docs.

RyanJField commented 2 years ago

I have also added a windows runner to the CI, currently testing on python 3.9 but this can be changed when #186 has been merged. Feel free to squash these commits.

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

bobturneruk commented 1 year ago

Win 10, Python 3.9.12

I was able to install using:

.\localregistry.bat -d C:\Users\bobturner\fair_registry -b hotfix/port-fix

I can start with:

start_fair_registry_windows.bat -p 8080 -a 127.0.0.1 -b

and see the server running in the browser, so a custom port is OK. 

.\stop_fair_registry_windows.bat

works.

However this:

start_fair_registry_windows.bat -p 8081 -a 192.168.0.15 -b

didn't start a server. Returns:

curl, Features: AsynchDNS HSTS IPv6 Kerberos Largefile NTLM SPNEGO SSL SSPI Unicode UnixSockets alt-svc libz is installed, continuing...
Reading Parameter -p...
Reading Parameter -a...
Reading Parameter -b...
Spawning Server at 192.168.0.15:8081
Writing Session and Port Info
waiting for server to start

So it's reading the IP and port correctly. I'm guessing this is actually fine as there's no server on 192.168.0.15 for it to run on - I don't know how django does it's business with this - I guess the -a flag is either for Docker somehow or if you want to deploy on an arbitrary server (that exists).

So looking good, I think. Before I leave a review - why do we have .bat and .ps1 files, please? I can't see where the .ps1 files are being used...

RyanJField commented 1 year ago

Win 10, Python 3.9.12

I was able to install using:

.\localregistry.bat -d C:\Users\bobturner\fair_registry -b hotfix/port-fix

I can start with:

start_fair_registry_windows.bat -p 8080 -a 127.0.0.1 -b

and see the server running in the browser, so a custom port is OK.

.\stop_fair_registry_windows.bat

works.

However this:

start_fair_registry_windows.bat -p 8081 -a 192.168.0.15 -b

didn't start a server. Returns:

curl, Features: AsynchDNS HSTS IPv6 Kerberos Largefile NTLM SPNEGO SSL SSPI Unicode UnixSockets alt-svc libz is installed, continuing...
Reading Parameter -p...
Reading Parameter -a...
Reading Parameter -b...
Spawning Server at 192.168.0.15:8081
Writing Session and Port Info
waiting for server to start

So it's reading the IP and port correctly. I'm guessing this is actually fine as there's no server on 192.168.0.15 for it to run on - I don't know how django does it's business with this - I guess the -a flag is either for Docker somehow or if you want to deploy on an arbitrary server (that exists).

So looking good, I think. Before I leave a review - why do we have .bat and .ps1 files, please? I can't see where the .ps1 files are being used...

The ps1 files offer an alternative to the batch scripts on windows. On the off chance you can't run batch scripts (remote servers which only offer powershell...). The default behaviour should be the batch scripts but the powershell scripts offer more compatability.

As for the server not running on a specific address this is likely a firewall issue.

bobturneruk commented 1 year ago

Makes sense. PowerShell versions seem to work the same for me.