Do not create hash conflicts #57

kzscisoft · 2021-09-01T09:07:43Z

Closes fair pull fails if you try to register a file with a hash already in the registry #59
Various other bugfixes

richardreeve

I think maybe this goes too far in the passive direction - we literally want it to do everything (set environment variable, create all of the necessary things(?) in the registry, create a working config and script) except run the script.

I thought it might be useful if it returned the path the config and script folder instead of running the script in fact, so that the output of fair run --ci could be directly used to find the files?

fair/run.py

kzscisoft · 2021-09-01T09:33:17Z

I think maybe this goes too far in the passive direction - we literally want it to do everything (set environment variable, create all of the necessary things(?) in the registry, create a working config and script) except run the script.

I thought it might be useful if it returned the path the config and script folder instead of running the script in fact, so that the output of fair run --ci could be directly used to find the files?

It does exactly the same as @DennisReddyhoff solution. All it is doing is skipping execution of an actual bash command, the rest is the same.

richardreeve · 2021-09-01T09:48:30Z

It does exactly the same as @DennisReddyhoff solution. All it is doing is skipping execution of an actual bash command, the rest is the same.

Great. Could we add in printing the folder path so that it can be picked up by any script calling it? That wasn't in the original solution, but I think it could be helpful to save investigating environment variables.

kzscisoft · 2021-09-01T10:26:01Z

It does exactly the same as @DennisReddyhoff solution. All it is doing is skipping execution of an actual bash command, the rest is the same.

Great. Could we add in printing the folder path so that it can be picked up by any script calling it? That wasn't in the original solution, but I think it could be helpful to save investigating environment variables.

Let's definitely discuss this later

* rr/fixes: Revert bodge to get local uri. Linting README Fix docs Fix initialisation from file Remove description key from some other unnecessary locations in code Replace --glob with --global

richardreeve · 2021-09-01T15:14:22Z

Okay. So I get this, which is great - with a config script of Sonia's that registers a few files:

(fair-6UDMIGpx-py3.8) (base) bleaberry[~/Documents/git/julia/EcoSISTEM] fair pull sonia-config1.yaml
Updating registry from config.yaml
(fair-6UDMIGpx-py3.8) (base) bleaberry[~/Documents/git/julia/EcoSISTEM] fair pull sonia-config1.yaml
Updating registry from config.yaml
Skipping item 'disease/sars_cov2/simple_model/parameters/static_params' as a hash matched entry is already present with this name
Skipping item 'disease/sars_cov2/simple_model/parameters/rts' as a hash matched entry is already present with this name
Skipping item 'disease/sars_cov2/simple_model/parameters/efoi' as a hash matched entry is already present with this name

However, when I call fair run it's less happy:

(fair-6UDMIGpx-py3.8) (base) bleaberry[~/Documents/git/julia/EcoSISTEM] fair run --ci sonia-config1.yaml
Updating registry from config.yaml
InternalError: Expected singular result for version '0.20210901.0'
(fair-6UDMIGpx-py3.8) (base) bleaberry[~/Documents/git/julia/EcoSISTEM] fair run --ci --debug sonia-config1.yaml
DEBUG:FAIRDataPipeline:Starting new session.
DEBUG:FAIRDataPipeline:Setting up command execution
Updating registry from config.yaml
DEBUG:FAIRDataPipeline.Run:Run called in passive mode, no command will be executed
DEBUG:FAIRDataPipeline.Run:Using job directory: /Users/richardr/.fair/data/jobs
DEBUG:FAIRDataPipeline.Run:Performing 'register' substitutions
Traceback (most recent call last):
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/bin/fair", line 5, in <module>
    cli()
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 1137, in __call__
    return self.main(*args, **kwargs)
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 1062, in main
    rv = self.invoke(ctx)
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 1668, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 763, in invoke
    return __callback(*args, **kwargs)
  File "/Users/richardr/tmp/FAIR-CLI/fair/cli.py", line 309, in run
    raise e
  File "/Users/richardr/tmp/FAIR-CLI/fair/cli.py", line 304, in run
    _hash = fair_session.run_job(script, mode=_run_mode)
  File "/Users/richardr/tmp/FAIR-CLI/fair/session.py", line 262, in run_job
    _hash = fdp_run.run_command(
  File "/Users/richardr/tmp/FAIR-CLI/fair/run.py", line 211, in run_command
    _conf_yaml = fdp_reg.subst_registrations(local_uri, _work_cfg)
  File "/Users/richardr/tmp/FAIR-CLI/fair/register.py", line 294, in subst_registrations
    raise fdp_exc.InternalError(
fair.exceptions.InternalError: Expected singular result for version '0.20210901.0'

I've put the config file itself on Zulip - here.

richardreeve · 2021-09-01T15:22:28Z

@kzscisoft As well as erroring, fair run generates the wrong working config file. It just outputs the original register block I think:

register:
- alternate_identifier_type: simple_model_params
  description: Static parameters of the model
  external_object: disease/sars_cov2/simple_model/parameters/static_params
  file_type: csv
  namespace_full_name: Biomathematics and Statistics Scotland
  namespace_name: BIOSS
  namespace_website: https://www.bioss.ac.uk
  path: FAIRDataPipeline/SimpleModel/main/inst/extdata/static_params_SEInRD.csv
  primary: true
  release_date: 2021-09-01 16:20:26
  root: https://github.com/
  title: Static parameters of the model
  unique_name: Simple model parameters - Static parameters of the model
  version: 0.20210901.0
- alternate_identifier_type: simple_model_params
  description: Values of Rt at time t
  external_object: disease/sars_cov2/simple_model/parameters/rts
  file_type: csv
  namespace_full_name: Biomathematics and Statistics Scotland
  namespace_name: BIOSS
  namespace_website: https://www.bioss.ac.uk
  path: FAIRDataPipeline/SimpleModel/main/inst/extdata/Rt_beep.csv
  primary: true
  release_date: 2021-09-01 16:20:26
  root: https://github.com/
  title: Values of Rt at time t
  unique_name: Simple model parameters - Values of Rt at time t
  version: 0.20210901.0
- alternate_identifier_type: simple_model_params
  description: Effective force of infection at time t
  external_object: disease/sars_cov2/simple_model/parameters/efoi
  file_type: csv
  namespace_full_name: Biomathematics and Statistics Scotland
  namespace_name: BIOSS
  namespace_website: https://www.bioss.ac.uk
  path: FAIRDataPipeline/SimpleModel/main/inst/extdata/efoi_all_dates.csv
  primary: true
  release_date: 2021-09-01 16:20:26
  root: https://github.com/
  title: Effective force of infection at time t
  unique_name: Simple model parameters - Effective force of infection
  version: 0.20210901.0
run_metadata:
  default_input_namespace: richardr
  default_output_namespace: richardr
  description: Simple Model
  latest_commit: 6d34068e0032f4aad5b1969f1b5c7aba75bf60b1
  local_data_registry_url: http://localhost:8000/api/
  local_repo: /Users/richardr/Documents/git/julia/EcoSISTEM
  public: true
  remote_data_registry_url: https://data.scrc.uk/api/
  remote_repo: https://github.com/EcoJulia/EcoSISTEM.jl.git
  script: echo "Hello World!"
  write_data_store: /Users/richardr/.fair/data

* dev: Bug fixes (#55)

* dev: Fix run --ci (PASS) mode to still generate the correct config and script files. Fixing output from `fair run --ci`. Print job directory in CI mode # Conflicts: # fair/cli.py

richardreeve · 2021-09-01T17:34:38Z

Okay, I've reconstructed and merged the original version of this PR, and then merged dev back in to reduce the size of this one. There may / are problems with the old PR, but this is a cleaner thing to check over, and then we can identify new bugs.

sonarcloud · 2021-09-02T02:33:05Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

richardreeve · 2021-09-02T02:36:45Z

Okay, all of the issues I can see are now fixed. There are still problems with uniquely identifying items in the registry, but there are no regressions as a result of this PR, so I think it can be merged.

kzscisoft · 2021-09-02T07:21:41Z

Okay. So I get this, which is great - with a config script of Sonia's that registers a few files:

(fair-6UDMIGpx-py3.8) (base) bleaberry[~/Documents/git/julia/EcoSISTEM] fair pull sonia-config1.yaml
Updating registry from config.yaml
(fair-6UDMIGpx-py3.8) (base) bleaberry[~/Documents/git/julia/EcoSISTEM] fair pull sonia-config1.yaml
Updating registry from config.yaml
Skipping item 'disease/sars_cov2/simple_model/parameters/static_params' as a hash matched entry is already present with this name
Skipping item 'disease/sars_cov2/simple_model/parameters/rts' as a hash matched entry is already present with this name
Skipping item 'disease/sars_cov2/simple_model/parameters/efoi' as a hash matched entry is already present with this name

However, when I call fair run it's less happy:

(fair-6UDMIGpx-py3.8) (base) bleaberry[~/Documents/git/julia/EcoSISTEM] fair run --ci sonia-config1.yaml
Updating registry from config.yaml
InternalError: Expected singular result for version '0.20210901.0'
(fair-6UDMIGpx-py3.8) (base) bleaberry[~/Documents/git/julia/EcoSISTEM] fair run --ci --debug sonia-config1.yaml
DEBUG:FAIRDataPipeline:Starting new session.
DEBUG:FAIRDataPipeline:Setting up command execution
Updating registry from config.yaml
DEBUG:FAIRDataPipeline.Run:Run called in passive mode, no command will be executed
DEBUG:FAIRDataPipeline.Run:Using job directory: /Users/richardr/.fair/data/jobs
DEBUG:FAIRDataPipeline.Run:Performing 'register' substitutions
Traceback (most recent call last):
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/bin/fair", line 5, in <module>
    cli()
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 1137, in __call__
    return self.main(*args, **kwargs)
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 1062, in main
    rv = self.invoke(ctx)
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 1668, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/richardr/Library/Caches/pypoetry/virtualenvs/fair-6UDMIGpx-py3.8/lib/python3.8/site-packages/click/core.py", line 763, in invoke
    return __callback(*args, **kwargs)
  File "/Users/richardr/tmp/FAIR-CLI/fair/cli.py", line 309, in run
    raise e
  File "/Users/richardr/tmp/FAIR-CLI/fair/cli.py", line 304, in run
    _hash = fair_session.run_job(script, mode=_run_mode)
  File "/Users/richardr/tmp/FAIR-CLI/fair/session.py", line 262, in run_job
    _hash = fdp_run.run_command(
  File "/Users/richardr/tmp/FAIR-CLI/fair/run.py", line 211, in run_command
    _conf_yaml = fdp_reg.subst_registrations(local_uri, _work_cfg)
  File "/Users/richardr/tmp/FAIR-CLI/fair/register.py", line 294, in subst_registrations
    raise fdp_exc.InternalError(
fair.exceptions.InternalError: Expected singular result for version '0.20210901.0'

I've put the config file itself on Zulip - here.

I think suggests that the objects are not being registered correctly.

richardreeve · 2021-09-02T07:32:08Z

I think I've fixed these problems now, but also note that there were some small issues with the registry itself, so you need to download a new copy - the latest is 1.0-rc4.

richardreeve · 2021-09-02T07:51:54Z

@kzscisoft Are you checking this now, or are you happy for me to merge it?

kzscisoft added the enhancement New feature or request label Sep 1, 2021

kzscisoft mentioned this pull request Sep 1, 2021

Add --ci option to fair run #53

Closed

richardreeve reviewed Sep 1, 2021

View reviewed changes

fair/run.py Outdated Show resolved Hide resolved

fair/run.py Outdated Show resolved Hide resolved

Fix version number for wildcards

fda06c4

kzscisoft mentioned this pull request Sep 1, 2021

Fixes wildcard statements not starting at 0.0.1 #60

Closed

Do not fail storage of a DATA file if hash exists

957dd28

kzscisoft changed the title ~~Fix logging and use CMD_MODE for passive run~~ Fix logging and use CMD_MODE for passive run, do not create hash conflicts and do not create config outside of command run Sep 1, 2021

Print job directory in CI mode

15fe2eb

kzscisoft requested a review from richardreeve September 1, 2021 14:37

Merge branch 'rr/fixes' into kzscisoft/ci-run

9413341

* rr/fixes: Revert bodge to get local uri. Linting README Fix docs Fix initialisation from file Remove description key from some other unnecessary locations in code Replace --glob with --global

richardreeve added 2 commits September 1, 2021 16:46

Merge branch 'dev' into kzscisoft/ci-run

275a6fa

* dev: Bug fixes (#55)

Merge branch 'dev' into kzscisoft/ci-run

bd9261c

* dev: Fix run --ci (PASS) mode to still generate the correct config and script files. Fixing output from `fair run --ci`. Print job directory in CI mode # Conflicts: # fair/cli.py

richardreeve changed the title ~~Fix logging and use CMD_MODE for passive run, do not create hash conflicts and do not create config outside of command run~~ Do not create hash conflicts Sep 1, 2021

richardreeve added 4 commits September 2, 2021 01:19

Fix failure to isolate just one data product in search

4f9e233

Fix write globbing?

6427628

Fix read globbing?

51e7861

Be sure to error when reading non-existent entry

5d5012b

richardreeve approved these changes Sep 2, 2021

View reviewed changes

kzscisoft merged commit 7f22e64 into dev Sep 2, 2021

richardreeve mentioned this pull request Sep 2, 2021

Wildcard versioning not working correctly #44

Closed

kzscisoft deleted the kzscisoft/ci-run branch October 22, 2021 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not create hash conflicts #57

Do not create hash conflicts #57

kzscisoft commented Sep 1, 2021 •

edited by richardreeve

Loading

richardreeve left a comment

kzscisoft commented Sep 1, 2021

richardreeve commented Sep 1, 2021

kzscisoft commented Sep 1, 2021

richardreeve commented Sep 1, 2021

richardreeve commented Sep 1, 2021

richardreeve commented Sep 1, 2021 •

edited

Loading

sonarcloud bot commented Sep 2, 2021

richardreeve commented Sep 2, 2021

kzscisoft commented Sep 2, 2021 •

edited

Loading

richardreeve commented Sep 2, 2021

richardreeve commented Sep 2, 2021

Do not create hash conflicts #57

Do not create hash conflicts #57

Conversation

kzscisoft commented Sep 1, 2021 • edited by richardreeve Loading

richardreeve left a comment

Choose a reason for hiding this comment

kzscisoft commented Sep 1, 2021

richardreeve commented Sep 1, 2021

kzscisoft commented Sep 1, 2021

richardreeve commented Sep 1, 2021

richardreeve commented Sep 1, 2021

richardreeve commented Sep 1, 2021 • edited Loading

sonarcloud bot commented Sep 2, 2021

richardreeve commented Sep 2, 2021

kzscisoft commented Sep 2, 2021 • edited Loading

richardreeve commented Sep 2, 2021

richardreeve commented Sep 2, 2021

kzscisoft commented Sep 1, 2021 •

edited by richardreeve

Loading

richardreeve commented Sep 1, 2021 •

edited

Loading

kzscisoft commented Sep 2, 2021 •

edited

Loading