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.

Sign up for GitHub

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

Jump to bottom

Broadcast: GraphQLLocatedError 'DurationFloat' object has no attribute 'startswith' #6334

Closed
ColemanTom opened this issue Aug 28, 2024 · 9 comments · Fixed by #6335
Closed

Broadcast: GraphQLLocatedError 'DurationFloat' object has no attribute 'startswith' #6334

ColemanTom opened this issue Aug 28, 2024 · 9 comments · Fixed by #6335
Assignees
@oliver-sanders
Labels
bug Something is wrong :(
Milestone
8.3.4

Comments

@ColemanTom
Copy link
Contributor

ColemanTom commented Aug 28, 2024 •
edited
Loading

Description

When broadcasting to set the execution time limit I'm seeing the error ERROR: [{'error': {'message': "'DurationFloat' object has no attribute 'startswith'", 'traceback': ["graphql.error.located_error.GraphQLLocatedError: 'DurationFloat' object has no attribute 'startswith'\n"]}}]. This may be related to #6333 and #6222
This happens if you have two or more namespaces and are using a ISO duration item.

Reproducible Example

$ cylc broadcast -s 'execution time limit=PT1M' -n root -n root access_g4_rewind
ERROR: [{'error': {'message': "'DurationFloat' object has no attribute 'startswith'", 'traceback': ["graphql.error.located_error.GraphQLLocatedError: 'DurationFloat' object has no attribute 'startswith'\n"]}}]

The namespaces can be the same, it doesn't matter. If you have one namespace, there are no issues, 2 it fails.

Expected Behaviour

No failure
@ColemanTom ColemanTom added the bug Something is wrong :( label Aug 28, 2024
@oliver-sanders
Copy link
Member

@ColemanTom, the issue here appears to be the duplicate -n root option. When I remove this, the command succeeds:

$ cylc broadcast -s 'execution time limit=PT1M' -n root -n root generic         
ERROR: [{'error': {'message': "'DurationFloat' object has no attribute 'startswith'", 'traceback': ["graphql.error.located_error.GraphQLLocatedError: 'DurationFloat' object has no attribute 'startswith'\n"]}}]
$ cylc broadcast -s 'execution time limit=PT1M' -n root generic
Broadcast set:
  [*/root] execution time limit=PT1M

oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Aug 28, 2024
@oliver-sanders
broadcast: remove duplicate namespaces
8520f99 
* Specifying the same namesapce multiple times can cause cryptic errors.
* Closes cylc#6334
@ColemanTom
Copy link
Contributor Author

The duplicate is for demonstration. You can do any combination of things, just 2 namespaces setting at least execution time limit, causes this failure.

@oliver-sanders oliver-sanders mentioned this issue Aug 28, 2024
Merged
8 tasks
@oliver-sanders oliver-sanders added this to the 8.3.4 milestone Aug 28, 2024
@oliver-sanders oliver-sanders self-assigned this Aug 28, 2024
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Aug 28, 2024
@oliver-sanders
broadcast: remove duplicate namespaces
398b8b9 
* Fix an issue that could cause issues when broadcasting "coerced"
  configurations to multiple namespaces.
* Specifying the same namesapce multiple times doesn't make sense,
  we should strip duplicates earlier on in the process.
* Closes cylc#6334
@oliver-sanders oliver-sanders linked a pull request Aug 28, 2024 that will close this issue
broadcast: fix issue with multiple namespaces #6335
Merged
8 tasks
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Aug 28, 2024
@oliver-sanders
broadcast: remove duplicate namespaces
eeb3205 
* Fix an issue that could cause issues when broadcasting "coerced"
  configurations to multiple namespaces.
* Specifying the same namesapce multiple times doesn't make sense,
  we should strip duplicates earlier on in the process.
* Closes cylc#6334
@oliver-sanders
Copy link
Member

Should be fixed by #6335. If you are able to test this it would help us get it in sooner.

@oliver-sanders oliver-sanders mentioned this issue Aug 28, 2024
Closed
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Aug 28, 2024
@oliver-sanders
broadcast: remove duplicate namespaces
24193d6 
* Fix an issue that could cause issues when broadcasting "coerced"
  configurations to multiple namespaces.
* Specifying the same namesapce multiple times doesn't make sense,
  we should strip duplicates earlier on in the process.
* Closes cylc#6334
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Aug 28, 2024
@oliver-sanders
broadcast: remove duplicate namespaces
5df5253 
* Fix an issue that could cause issues when broadcasting "coerced"
  configurations to multiple namespaces.
* Specifying the same namesapce multiple times doesn't make sense,
  we should strip duplicates earlier on in the process.
* Closes cylc#6334
@ColemanTom
Copy link
Contributor Author

As I said, the root duplication was me demonstrating without knowing any namespaces. Here is a simple example.

$ cat flow.cylc
[scheduling]
    initial cycle point = 21200101T0000Z
    final cycle point = 21200101T0000Z
    [[graph]]
        PT6H = """
                @wall_clock => start & end
        """
[runtime]
    [[start,end]]
        script = true

$ cylc broadcast -s 'execution time limit=PT9M' -n start -n end basic
ERROR: [{'error': {'message': "'DurationFloat' object has no attribute 'startswith'", 'traceback': ["graphql.error.located_error.GraphQLLocatedError: 'DurationFloat' object has no attribute 'startswith'\n"]}}]

It is nothing to do with duplicate namespaces. Cylc (or something) must be resolving the PT9M to be a DurationFloat, which breaks GraphQL.

@ColemanTom
Copy link
Contributor Author

I have run with your change, and same error by the way.

@oliver-sanders
Copy link
Member

I have run with your change, and same error by the way.

I tried with your example above. Could you post some details of how you reproduced this on the PR so I can investigate, thanks.

Note, you will need to restart your workflows with the patched version because it's a server-side fix not a client-side one.

@ColemanTom
Copy link
Contributor Author

Ah, I missed updating broadcast.py yesterday. I had only updated broadcast_mgr.py. With your new broadcast.py in there too, the error has disappeared, whether the namespaces mentioned are duplicate or not.

$ CYLC_VERSION=8.3.3 cylc broadcast -s 'execution time limit=PT9M' -n start -n end basic
Broadcast set:
  [*/end] execution time limit=PT9M
  [*/start] execution time limit=PT9M
$ CYLC_VERSION=8.3.3 cylc broadcast -s 'execution time limit=PT9M' -n start -n start basic
Broadcast set:
  [*/start] execution time limit=PT9M

So, this looks like it can be closed when #6335 is merged in.

@oliver-sanders
Copy link
Member

Thanks for confirming.

@oliver-sanders
Copy link
Member

Closed by #6335 (Cylc 8.3.4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oliver-sanders oliver-sanders

Labels
bug Something is wrong :(
Projects
None yet
Milestone
8.3.4
Development

Successfully merging a pull request may close this issue.

broadcast: fix issue with multiple namespaces oliver-sanders/cylc-flow
2 participants
@ColemanTom @oliver-sanders