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

Update Rubin ps-connector.properties file #231

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Conversation

hernandezc1
Copy link
Collaborator

sed -i "s/PS_TOPIC/${PS_TOPIC}/g" ${fconfig}

Line 71 in the VM's startup script, displayed above, updates the value PS_TOPIC in the ps-connector.properties file with the appropriate PubSub topic name. Due to a typo in the ps-connector.properties file, the startup script was unable to update the PS_TOPIC value. This PR fixes the typo.

Changes have been tested. No additional bugs detected

@hernandezc1 hernandezc1 added the Bug Something isn't working label Jun 7, 2024
@hernandezc1 hernandezc1 self-assigned this Jun 7, 2024
@hernandezc1 hernandezc1 requested a review from wmwv June 7, 2024 19:16
Copy link
Collaborator

@wmwv wmwv left a comment

Choose a reason for hiding this comment

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

This change is obviously a good fix.

@wmwv
Copy link
Collaborator

wmwv commented Jun 7, 2024

But more broadly, this sed command implies that PS_TOPIC can't have special characters (~, &, \1) that would might something in a regex or be interpreted by the sed command (/). Is that already a requirement by GCP, and is it already documented?

@hernandezc1
Copy link
Collaborator Author

hernandezc1 commented Jun 11, 2024

But more broadly, this sed command implies that PS_TOPIC can't have special characters (~, &, \1) that would might something in a regex or be interpreted by the sed command (/). Is that already a requirement by GCP, and is it already documented?

@wmwv GCP does have a guideline for naming a topic, subscription, schema, or snapshot. With the way Pub/Sub resources are currently created, I can see this becoming an issue if the testid contains a special character. However, our docs explicitly state that the testid is a string of lowercase letters and numbers, beginning with a letter.

@hernandezc1 hernandezc1 merged commit eca877a into develop Jun 11, 2024
4 checks passed
@hernandezc1 hernandezc1 deleted the u/ch/rubinvmbugfix branch June 11, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants