Skip to content

Conversation

antgonza
Copy link
Member

No description provided.

Copy link
Contributor

@AmandaBirmingham AmandaBirmingham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am very nervous about changing the expected test output without understanding why we are now getting a different path through the code than we were previously. Since I know you've spent considerable time looking at this, I have a couple of questions on this point.

# note for future reference
with self.assertRaisesRegex(
RuntimeError, "Request 'post https://localhost:21174/"
"qiita_db/prep_template/' did not succeed"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antonio, I see that the code that is actually throwing this new error hasn't changed in a while, so it is hard to figure out why it would suddenly start erroring. However, it looks like the code almost directly above it at line 618, that sets the prep file to be the original-prep.csv, is a quite recent addition. Is there any chance that the original-prep.csv, whatever it is, is missing out on some kind of name scrubbing that the previously-used files in self.prep_file_paths went through?

I am asking because the new error we are getting is (obviously) a correct one--there are sample names in the test input file that DO contain invalid characters. Thus, what I am really puzzled about is why we WEREN'T getting this error before ... where did the previous invalid input syntax for type uuid error get thrown when this test was working as expected? Was it upstream or downstream of the error we are seeing now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the problem could be. I have tried multiple times in my local installation and it works fine. I have a couple of ideas that I'm going to try.

Copy link
Contributor

@AmandaBirmingham AmandaBirmingham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WOW, I would definitely never have guessed that cause. Serious props for tracking that down!!

@antgonza antgonza merged commit be759c1 into qiita-spots:main Jul 25, 2025
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants