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

Possible bug: A send without critical files received #155

Open
k3a opened this issue Nov 6, 2024 · 5 comments
Open

Possible bug: A send without critical files received #155

k3a opened this issue Nov 6, 2024 · 5 comments

Comments

@k3a
Copy link

k3a commented Nov 6, 2024

I have been using segment_gatherer with ini config style and it has been working fine but sometimes I noticed it would send a dataset with just one file, especially after a restart. I have a list of critical files defined.

My config:

[msg]
pattern = H-000-{orig_platform_name:4s}__-{orig_platform_name:4s}________-{channel_name:_<9s}-{segment:_<9s}-{start_time:%Y%m%d%H%M}-{compression_suffix}
# Segments critical to production
critical_files = :PRO,:EPI,VIS006:000001-000008,VIS008:000001-000008,IR_016:000001-000008
# These segments we want to have, but it's still ok if they are missed
wanted_files = :PRO,:EPI,VIS006:000001-000008,VIS008:000001-000008,IR_016:000001-000008,HRV:000001-000024
# All possible segments
all_files = :PRO,:EPI,VIS006:000001-000008,VIS008:000001-000008,IR_016:000001-000008,IR_039:000001-000008,WV_062:000001-000008,WV_073:000001-000008,IR_087:000001-000008,IR_097:000001-000008,IR_108:000001-000008,IR_120:000001-000008,IR_134:000001-000008,HRV:000001-000024
topics = /input/meteosat
publish_topic = /dataset/meteosat
timeliness = 900
# Name of a time field in the pattern above
time_name = start_time
# Comma separated list of tag names in the pattern that vary between different segments of the same time slot
variable_tags = compression_suffix

By doing some debugging I found out that ini_to_dict conversion code sets is_critical_set to False:

patterns['is_critical_set'] = False

Is that really correct? In YAML it is possible to mark a set as critical or not and I can eventually migrate my config to it. But shouldn't all legacy ini sets be critical by default? 🤔

@mraspaud
Copy link
Member

mraspaud commented Nov 8, 2024

@k3a thanks for reporting this issue!
@pnuu do you have an opinon on this?

@pnuu
Copy link
Member

pnuu commented Nov 8, 2024

I think I saw this in the past, too. Currently I only have the YAML variants in use, so have forgotten about it.

I think the is_critical_set shouldn't be set to False or True if the option is configured in the config. If it is not, then I'm leaning to setting it to False by default so that there are at least some messages sent.

@k3a
Copy link
Author

k3a commented Nov 12, 2024

Are you sure?

The documentation defines:

critical_files

    Describes the files that must be unconditionally present. If timeout is reached
    and one or more critical files are missing, no message is published and
    all further processing ceases. 

If I understand the concept correctly, if someone wants "at least some messages sent" then they can simply not define critical_files option or am I missing something?

In YAML each set can have this flag is_critical_set specified separately like in this example

but in INI this is not possible so it seems strange to me that it should be False by default. Isn't this making critical_files in INI not working and simply useless?

Of course, I can migrate to YAML, no big deal but I am confused.

@mraspaud
Copy link
Member

The Ini format support was the first we implemented, but we later moved to yaml. So IMO the ini support should be deprecated.
My impression is that we never bothered implementing the critical set for ini, but if you need it, feel free to provide a pull request!

@gerritholl
Copy link
Member

I wrote that part of the documentation based on my understanding of until-then undocumented behaviour. It is possible that I misunderstood and that the documentation is incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants