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

refactor: datacollector #2240

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

refactor: datacollector #2240

wants to merge 3 commits into from

Conversation

PiTrem
Copy link
Member

@PiTrem PiTrem commented Nov 14, 2024

refactoring

Module Datacollector

separate business logic

  • collector.rb: file/folder crawler for a given device (connection independant)
  • correspondence.rb: set the sender and recipient encompass AR Model logic to create the attachments into the proper containers
  • configuration.rb: validate the datacollector device config also instantiate the SFTP client if applicable
  • collector_file: file/folder class for basic file operations independent of whether the file is on local fs or over sftp

SFTPClient

make use of the existing SFTP client wrapper that ensures the connection is closed, instead of initialising the session directly.

AdminApi device connection testing:

* now rely on Datacollector::Configuration

Next Time

  • DRY: Device attribute validation to use validation from Datacollector::Configuration
  • Validate Write permission on the watch dir
  • add a disabling option to collector methods

app/clients/sftp_client.rb Outdated Show resolved Hide resolved
app/clients/sftp_client.rb Show resolved Hide resolved
app/models/user.rb Outdated Show resolved Hide resolved
config/initializers/datacollectors.rb Show resolved Hide resolved
config/initializers/datacollectors.rb Show resolved Hide resolved
spec/lib/datacollector/correspondence_spec.rb Show resolved Hide resolved
spec/lib/datacollector/correspondence_spec.rb Outdated Show resolved Hide resolved
spec/lib/datacollector/correspondence_spec.rb Outdated Show resolved Hide resolved
spec/lib/datacollector/correspondence_spec.rb Outdated Show resolved Hide resolved
spec/lib/datacollector/correspondence_spec.rb Outdated Show resolved Hide resolved
@PiTrem PiTrem force-pushed the fix_data_collector branch 2 times, most recently from e592e1d to 96a198f Compare November 14, 2024 14:06
Module Datacollector
separate business logic
  * collector.rb: file/folder crawler for a given device
  * correspondence.rb: set the sender and recipient
     encompass AR Model logic to create the attachments into the proper
     containers
  * configuration.rb: validate the datacollector device config
     also instantiate the SFTP client if applicable
  * collector_file: file/folder class for basic file operations
      independent of whether the file is on local fs or over sftp

  AdminApi device connection testing:
    * now rely on Datacollector::Configuration
ext { 'chemotion' }
name { File.basename(Faker::File.file_name(ext: ext)) }
root { nil }
touch { true }

Choose a reason for hiding this comment

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

Rails/SkipsModelValidations: Avoid using touch because it skips validations.

initialize_with do
pathname = Pathname.new(prefix ? "#{prefix}-#{name}" : name)
pathname = root.join(pathname) if root
FileUtils.touch(pathname) if touch

Choose a reason for hiding this comment

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

Rails/SkipsModelValidations: Avoid using touch because it skips validations.

pathname = root.join(pathname) if root
FileUtils.touch(pathname) if touch
FileUtils.cp(copy_from, pathname) if copy_from
pathname.chmod(mode) if touch || copy_from

Choose a reason for hiding this comment

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

Rails/SkipsModelValidations: Avoid using touch because it skips validations.

@@ -1,5 +1,11 @@
# frozen_string_literal: true

DC_SFTP_USER = ENV['DATACOLLECTOR_FACTORY_SFTP_USER'].presence || `whoami`.strip
DC_DIR = ENV['DATACOLLECTOR_FACTORY_DIR'].presence ||
Rails.configuration.datacollectors.dig(:localcollectors, 0, :path)

Choose a reason for hiding this comment

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

Layout/MultilineOperationIndentation: Align the operands of an expression in an assignment spanning multiple lines.

datacollector_host { '127.0.0.1' }
datacollector_authentication { 'keyfile' }
datacollector_key_name { "#{Dir.home}/.ssh/id_test" }
datacollector_dir { File.join(DC_DIR, "#{name_abbreviation}-#{datacollector_method}#{datacollector_user_level_selected ? '-user' : ''}").to_s }

Choose a reason for hiding this comment

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

Layout/LineLength: Line is too long. [149/120]

@@ -1,5 +1,11 @@
# frozen_string_literal: true

DC_SFTP_USER = ENV['DATACOLLECTOR_FACTORY_SFTP_USER'].presence || 'testuser' #`whoami`.strip

Choose a reason for hiding this comment

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

Layout/LeadingCommentSpace: Missing space after #.

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.

1 participant