-
Notifications
You must be signed in to change notification settings - Fork 0
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
Streamline Generate #15
Conversation
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.
Generally looks pretty good. I have a number of code-quality suggestions and there is one area of code that I think needs some deeper thought: Manifold::API::Workspace#dimensions_fields
and friends.
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.
Looks great. I've made one further suggestion, but I'll let you decide whether to adopt it or not.
@logger.error("Vector configuration not found: #{path}") | ||
return nil | ||
end | ||
raise "Vector configuration not found: #{path}" unless path.file? |
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.
A non-existent or non-file file are not the only possible failure modes here. It could also be an invalid YAML file. Consider replacing this with rescues for Errno::ENOENT
, Errno::EISDIR
, and Psych::Exception
.
Refactors the
generate
interface to accept no arguments. You just callmanifold generate
to generate the manifolds for all workspaces in your project.I also factored out the BigQuery service. I think we can consider BigQuery the only destination we're generating things for for the foreseeable future and skip managing the complexity of multiple targets for now.
Copied over the general generation logic from the former BigQuery service into a
#generate
method on Workspaces, but oop'ified it.I also renamed the housing directory for workspaces, projects and vectors to
api
, reflecting the module hierarchy.