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

Add database afterCreate pool init hook #14310

Open
rijkvanzanten opened this issue Jul 7, 2022 Discussed in #14153 · 12 comments
Open

Add database afterCreate pool init hook #14310

rijkvanzanten opened this issue Jul 7, 2022 Discussed in #14153 · 12 comments

Comments

@rijkvanzanten
Copy link
Member

This will allow specific database connection params to be used, like WAL mode on Sqlite, or CockroachDB options like serial_normalization etc.

Discussed in #14153

Originally posted by xmichaelx June 28, 2022
Is Directus using WAL with SQLite backend? If not is it possible to enable it with executing pragma journal_mode=wal; somewhere? Can it be configured?

I see that in https://github.com/directus/directus/blob/895100898cd6f0f6121e964e793137059e82fc1b/api/src/database/index.ts
there is line which enables foreign_keys await run('PRAGMA foreign_keys = ON'); would it be possible to add WAL? or at least do so depending on additional variable?

@rijkvanzanten
Copy link
Member Author

rijkvanzanten commented Jul 7, 2022

Regarding implementation: right now we're only hooking into the afterCreate for sqlite and cockroach, as Directus needs to set a couple options to run. We should rework that a bit to have a single afterCreate call on knex, with if statements in there for the vendor specific stuff.

@rijkvanzanten rijkvanzanten removed their assignment Jul 7, 2022
@Vinayak119A
Copy link

Hey, I am Vinayak Kanjker I would like to contribute in this app can you please assign me a task .
This is for my open source contribution class.
please contact me at vinukanjiker@gmail.com

@rijkvanzanten
Copy link
Member Author

@Vinayak119A Wanna give this one a shot? 🙂

Also what is open source contribution class? Very curious to learn more about what class/study/program is teaching that 🙂

@Vinayak119A
Copy link

@rijkvanzanten Yes sir I would like to give this one a shot . May I contribute?

@rijkvanzanten
Copy link
Member Author

For sure! Give it a shot and lemme know how it goes in a PR :)

@Vinayak119A
Copy link

Vinayak119A commented Oct 9, 2022

@rijkvanzanten sir I have tried this code please accept the request

code to declare hook

// Method 1 via the .init() method
class User extends Model {}
User.init({
  username: DataTypes.STRING,
  mood: {
    type: DataTypes.ENUM,
    values: ['happy', 'sad', 'neutral']
  }
}, {
  hooks: {
    beforeValidate: (user, options) => {
      user.mood = 'happy';
    },
    afterValidate: (user, options) => {
      user.username = 'Toni';
    }
  },
  sequelize
});

// Method 2 via the .addHook() method
User.addHook('beforeValidate', (user, options) => {
  user.mood = 'happy';
});

User.addHook('afterValidate', 'someCustomName', (user, options) => {
  return Promise.reject(new Error("I'm afraid I can't let you do that!"));
});

// Method 3 via the direct method
User.beforeCreate(async (user, options) => {
  const hashedPassword = await hashPassword(user.password);
  user.password = hashedPassword;
});

User.afterValidate('myHookAfter', (user, options) => {
  user.username = 'Toni';
});

Adding database afterCreate pool init hook

router.post('/', function(req, res) {
//node return reply after running the create
  return models.User.create({ username: req.body.username }).then(function() {      
    return res.json({ message: 'New user created' });
  });
});

@rijkvanzanten
Copy link
Member Author

@Vinayak119A I ehh have no idea what we're looking at above, but it doesn't seem relevant / related to the Directus codebase at all.. 🤷🏻

@rijkvanzanten
Copy link
Member Author

Linear: ENG-235

@alexchudnovsky
Copy link

Hi @rijkvanzanten

As a temporary work around, there is no reason that WAL could not be enabled directly through something like DB Browser for SQLite right ?

Kind Regards,
Alex

@rijkvanzanten
Copy link
Member Author

@alexchudnovsky Hmm good question. I believe some SQLite settings happen on the connection level instead of the database level, so it depends on what you're configuring 👍🏻

@st17931
Copy link

st17931 commented Apr 4, 2023

@rijkvanzanten Is it still open.Can i start working on this as first time contributor in @directus.Else give me some good first issue which is still open.
Thanks.

@rijkvanzanten
Copy link
Member Author

Sure! Go for it @st17931

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Development

Successfully merging a pull request may close this issue.

5 participants