Skip to content

Conversation

@apdavison
Copy link
Member

CircleCI sometimes times out because no output is received for a long time while downloading files.
Here we try two approaches:

  • increase no_output_timeout in config.yml
  • use --debug flag to print output every time a file is downloaded

CircleCI sometimes times out because no output is received for a long time while downloading files.
Here we try two approaches:
- increase no_output_timeout in config.yml
- use --debug flag to print output every time a file is downloaded
@apdavison
Copy link
Member Author

@JuliaSprenger @samuelgarcia it would be good to merge this as soon as possible, if you see no problems with it. All of the recent CircleCI runs have been broken due to a time out. I think this might have been triggered by the recent addition of a lot of new Neuralynx test files, which take a long time to download, e.g.

https://app.circleci.com/pipelines/github/NeuralEnsemble/python-neo/222/workflows/f6eda479-b6c8-4651-ba61-5cb85c035ec7/jobs/1867

For some reason this PR didn't trigger a CircleCI run, but you can see here that the tests all pass on my branch (you'll also see all the "neo.test: INFO: Downloading https://web.gin.g-node.org...." messages, which prevent the time out).

https://app.circleci.com/pipelines/github/apdavison/python-neo/63/workflows/0c837825-6ad3-416b-9472-fe865e1f63e4/jobs/483

@JuliaSprenger JuliaSprenger mentioned this pull request Dec 7, 2020
@JuliaSprenger
Copy link
Member

Hi @apdavison !
Thanks for having a look into this issue. I agree this should be merged as soon as possible, but I would like to also make sure that CircleCi will still be triggered also with these changes. Opening #920 seems to have worked for this.

@samuelgarcia
Copy link
Contributor

This is OK for me.
Maybe we could have another command to download everything before the nosetetst.
This is strange because witth the cache the download should be quite small, no ?
We should normally download only what is news. Am I wrong ?

@apdavison
Copy link
Member Author

I think there's maybe a problem with the cache key. Because this is unchanged, the newly downloaded files don't get cached.

@samuelgarcia
Copy link
Contributor

Do we need to change theses lines:

- test-files-f7905c85d1

key: test-files-f7905c85d1

Who is generating the key. Us or circle ?

@apdavison
Copy link
Member Author

Us. See https://circleci.com/docs/2.0/caching/

I think we need to add "test-files-" to the keys for restore_cache, and we should change the hex suffix for the save_cache.

Ideally we should create the key algorithmically. One way would be to move the list of test files out of the Python code into an external YAML or JSON file, and we could then use a hash of the contents of that file as the key suffix.

Also note that caches are only retained for 15 days, so if there are no PRs or updates within a 15-day period the files will have to be downloaded again.

@samuelgarcia
Copy link
Contributor

Yes a centralised yaml sounds good.
But the way it is done now is like this for instance with TestNeuroScopeRawIO

    files_to_download = ['test1/test1.xml',
                         'test1/test1.dat',
                         ]
    entities_to_test = ['test1/test1']

If we move the files_to_download to yaml then we have to maintain at 2 places one for files_to_download and one for entities_to_test.

Maybe we could also make a script that parse all our test files. Construct this yaml on the fly. And make the hash on the fly.

@samuelgarcia
Copy link
Contributor

Do you want I merge this PR now ?

@apdavison
Copy link
Member Author

Rather than parse the files and construct on the fly, how about a yaml file structured like:

- SomeIO
    - test1/test1               # this is the entity to test
        - test1/test1.xml       # } these are the files to download
        - test1/test1.dat       # } for that entity

@samuelgarcia
Copy link
Contributor

Good idea.
I merge and create an issue for the yaml.

@samuelgarcia samuelgarcia merged commit 37b9012 into NeuralEnsemble:master Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants