-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix exceptions, mark required options as required #104
Conversation
The bot and the webhooks are both down due to this, merging to address emergency circumstances... |
def sqs_message_attribs(self, ckan_group=None): | ||
attribs = {} | ||
if ckan_group and not getattr(self, 'x_netkan_allow_out_of_order', False): | ||
attribs['HighestVersion'] = ckan_group.highest_version().string | ||
attribs['HighestVersion'] = self.string_attrib(ckan_group.highest_version().string) | ||
return attribs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should open an issue to refactor this so that it does the RightThings™ - There is definitely a nicer way to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this was just a safe short term fix to get it running again, please feel free to change it to whatever would be best.
@@ -618,6 +618,7 @@ | |||
'env': [ | |||
('SQS_QUEUE', GetAtt(inbound, 'QueueName')), | |||
('NETKAN_REMOTE', NETKAN_REMOTE), | |||
('CKANMETA_REMOTE', CKANMETA_REMOTE), | |||
('AWS_DEFAULT_REGION', Sub('${AWS::Region}')), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I better deploy this!
I have started looking at writing some decorators to make things like this far less likely to be missed. But I'll get the stack updated shortly. |
Problems
Cause
The
--ckanmeta-remote
parameter isn't being passed to the scheduler (becausethe stack needs to be updatedthe environment variable was missing for the web hooks pass), but it's running anyway. This is a problem because that parameter is needed for the scheduler to do its work. There are some other instances of this problem incli.py
.The
HighestVersion
message attribute from #102 was passed as a string. Apparently it's supposed to be a dict:https://boto3.amazonaws.com/v1/documentation/api/latest/guide/sqs.html
Changes
Now all required Click options in
cli.py
are marked as required. As well, some minor formatting tweaks (trailing commas) are made for consistency and ease of maintenance.Now
CKANMETA_REMOTE
is passed to the scheduler web hooks pass, which will fix the root cause of the above error.Now we generate a dict for
HighestVersion
.