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.

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

process.executor is not recognized for certain nextflow versions and process orders in the configs #2966

Closed
replikation opened this issue Jun 16, 2022 · 11 comments

Comments

@replikation
Copy link

replikation commented Jun 16, 2022

Bug report

Expected behavior and actual behavior

TLDR: process.executor = 'google-lifesciences' is not recognized for certain nextflow versions if its written before another process {} statment.

We have certain google life science workflows that run without issues with nextflow version nextflow.21.06.
We noticed that with the newer nextflow versions these workflows are running locally and not in the cloud. with e.g. nextflow.22.06.0-edge or 22.04.3 build 5703. After trying out simplistic mock workflows I could narrow it down to the config file and the order process code appears.

Steps to reproduce the problem

The below config examples were simplified down but still tested to reproduce the bug. We normally use includeConfig statements instead of multiple process statements in one file.

NOT WORKING: This configuration does NOT run in the cloud with process.executor before the additional process { } statment. It runs it locally

profiles {
    cloud { 

        process.executor = 'google-lifesciences'

	process {
    		withLabel:  ubuntu       { container = 'nanozoo/template:3.8--e13dfeb' }
	}
	
        workDir = "/tmp/nextflow"
        docker { enabled = true }
        bucketDir = '***'
        google {  
            project = '***'
            zone = 'europe-west1-b'
        }
    }
}

(replaced google infos with **** but they are the same between both configs)

WORKING: This configuration runs fine in the cloud with the process.executor after the additional process { } statment.

profiles {
    cloud { 

	process {
    		withLabel:  ubuntu       { container = 'nanozoo/template:3.8--e13dfeb' }
	}

        process.executor = 'google-lifesciences'
	
        workDir = "/tmp/nextflow"
        docker { enabled = true }
        bucketDir = '***'
        google {  
            project = '***'
            zone = 'europe-west1-b'
        }
    }
}

Additionally, this issue seems only to occur with a process { } not with single-line statements like process. Moreover in the working example the withLabel: ubuntu is recognized as the cloud needs a container to run or it errors.

Program output

no errors, it's just not recognizing the executor statement?

Environment

bug NOT in nextflow.21.06.
bug detected in nextflow.22.06.0-edge an 22.04.3 build 5703
no other version were tested

  • Java version: openjdk 11.0.15 2022-04-19
  • Operating system: Ubuntu 20.04.4 LTS
  • Bash version: GNU bash, version 5.0.17(1)-release (x86_64-pc-linux-gnu)
@jorgeaguileraseqera
Copy link
Contributor

Yes, this is a known issue

To cover corner cases in configuration ( see #2701 for example) ConfigParser has been changed.

The main problem is dot annotation is evaluated by the compiler in a different way than closure annotation so the best approach is not to mix both of them or use first the closure annotation to initialize the config map properly

sorry for the inconvenience.

Probably documentation needs to be improved to explain this issue

@replikation
Copy link
Author

replikation commented Jun 17, 2022

Thanks for responding.

I see #2701 has a similar problem. We are managing several config files across multiple profile to reduce the Code management (e.g. a container config to just specifiy images shared across a few profiles). The new nextflow versions now make this difficult especially as no warning or information is provided that certain statements are ignored.

Does this mean this bug won't be fixed? This would make configurations really unreliable if some of them are ignored.

@phiweger
Copy link

@jorgeaguileraseqera in the future, would not a simple json formatted config make this easier/ more reliable? #featureRequest

@jorgeaguileraseqera
Copy link
Contributor

@jorgeaguileraseqera in the future, would not a simple json formatted config make this easier/ more reliable? #featureRequest

@phiweger is not only about the format. Json, Yaml or Toml are valid format (in fact there is an issue created about them) but also include configurations, interpolation, variable resolution and so on

@jorgeaguileraseqera
Copy link
Contributor

Does this mean this bug won't be fixed? This would make configurations really unreliable if some of them are ignored.

@replikation no, no, my comment was more as providing a workaround (move dot annotation after closure) meanwhile the issue is investigated

@replikation
Copy link
Author

@jorgeaguileraseqera thanks for clarifying.

@bentsherman
Copy link
Member

There is a warning about this issue in the docs: https://www.nextflow.io/docs/latest/config.html#config-profiles

But the warning talks specifically about profiles whereas it seems the error happens regardless of whether the process block is inside a profile.

@replikation
Copy link
Author

Can't see how this applies as I specified the executer only once. I don't know so this might be attributed to nextflow automatically specifying the Local executer or so?

@bentsherman
Copy link
Member

The linked warning shows the same problem explained by Jorge, defining process settings first by dot notation and then by closure. Just putting it here to help anyone who comes across this thread. I suspect this problem will need to be solved as part of a larger overhaul of the config system. For now the workaround is to move the dot notation settings after the closure settings, or avoid using both at the same time.

@stale
Copy link

stale bot commented Jan 16, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 16, 2023
@stale stale bot closed this as completed Mar 18, 2023
@phiweger
Copy link

reupping this

@bentsherman bentsherman added pinned and removed stale labels May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants