SNEWS2 / SNEWS_Publishing_Tools

Publishing Tool for SNEWS
BSD 3-Clause "New" or "Revised" License
2 stars 2 forks source link

Test broker #67

Closed KaraMelih closed 1 year ago

joesmolsky commented 1 year ago

I didn't review the notebook.

In main.py, I think the --start-at flag is confusing. The help string options are "LATEST" or "EARLIEST". The docstring in test_connection describes it as a negative int in main.py and a string in remote commands.

I think this may just be a docstring typo in main.py. I'm also not clear on the use case for the "EARLIEST" option, but it doesn't seem to hurt anything.

KaraMelih commented 1 year ago

Thanks Joe! I fixed that docstring. It looks like hop allows for start_at='EARLIEST' or start_at='LATEST' options. When it starts from the earliest, it reads all the messages in send to broker (not clear how long back this goes) When you say latest it should be looking at the latest message sent to that broker. (Again not clear from their docstring if this looks at only the latest and exits or starts looking from the latest and goes backwards -which would be preferred as we break once the confirmation message is found)

joesmolsky commented 1 year ago

It seems I have access to the test-connection topic but I am still not able to get a connection confirmation message. I tried with --firedril and --no-firedrill. Also, the --wait option doesn't seem to do anything. It may be things are always timing out before the message can be confirmed. The following only took about 3-5 seconds to run.

❯ snews_pt test-connection --wait 5000

> Testing your connection.
> Sending to kafka://kafka.scimma.org/snews.experiments-firedrill
> Expecting from kafka://kafka.scimma.org/snews.connection-testing. 
> Should take ~5000 seconds...

        Couldn't get a confirmation in 5000 sec. 
        Maybe increase timeout and try again.

--start_at still only accepts integers from the command line:

Error: Invalid value for '--start_at' / '-s': 'LATEST' is not a valid integer.

The output from snews_pt test-connection --help need updating too.

KaraMelih commented 1 year ago

Screenshot 2023-05-08 143615 There is no longer a wait option, but rather a patience and the start_at issue must be fixed, it is looking for a string now. Since I merged it to main, could you switch branches on your local machine, pull from main and try again (using --no-firedrill)?